Add SecureUser ID Store #126
Conversation
| 'email' => User::EMAIL, | ||
| 'username' => User::USERNAME, | ||
| 'locked' => User::LOCKED, | ||
| 'manager_email' => User::MANAGER_EMAIL, |
There was a problem hiding this comment.
Is this complete? I thought Schram added 2 manager fields. Just checking.
There was a problem hiding this comment.
I think Schram's addition was just to the workday store for adding the HR fields.
There was a problem hiding this comment.
Yep. Not all fields must be listed here.
| */ | ||
| public function getUsersChangedSince(int $unixTimestamp) | ||
| { | ||
| throw new Exception(__FUNCTION__ . ' not yet implemented'); |
There was a problem hiding this comment.
Should we be pushing code with "not yet implemented" things?
There was a problem hiding this comment.
It's ok because configuration dictates which features are needed for a given ID Store. This method is only used for incremental sync. Presumably, secureuser is fine with running full sync each time.
| 'apiSecret', | ||
| ]; | ||
| foreach ($requiredProperties as $requiredProperty) { | ||
| if (empty($this->$requiredProperty)) { |
There was a problem hiding this comment.
Is this the PHP Magic which will use the new ID_STORE_CONFIG variables?
There was a problem hiding this comment.
application/common/config/main.php line 20
$idStoreOptionalConfig = Env::getArrayFromPrefix('ID_STORE_CONFIG_');
and then lines 73-77
'idStore' => ArrayHelper::merge([
'class' => IdStoreBase::getAdapterClassFor(
Env::get('ID_STORE_ADAPTER')
),
], $idStoreOptionalConfig),
| { | ||
| $client = $this->getHttpClient(); | ||
|
|
||
| $api_sig = hash_hmac('sha256', time().$this->apiKey, $this->apiSecret); |
There was a problem hiding this comment.
Is this right? time().$this->apiKey
There was a problem hiding this comment.
Yep. Concatenation of the time and the api key.
There was a problem hiding this comment.
Oh, haha, I was reading the . as if it was Go. I think maybe spaces on each side of the . are more standard.
| $api_sig = hash_hmac('sha256', time().$this->apiKey, $this->apiSecret); | |
| $api_sig = hash_hmac('sha256', time() . $this->apiKey, $this->apiSecret); |
| public function iAskTheIdStoreForASpecificActiveUser() | ||
| { | ||
| $this->activeEmployeeId = Env::requireEnv('TEST_SECURE_USER_EMPLOYEE_ID'); | ||
| $this->result = $this->idStore->getActiveUser($this->activeEmployeeId); |
There was a problem hiding this comment.
From what I can see, this method is only used for incremental sync. Do you still want to include it in the test? And maybe include a test for getAllActiveUsers?
There was a problem hiding this comment.
Since getActiveUser calls getAllActiveUsers, this is testing both in a way. I didn't want the test to have to validate a long list of users. As long as one known user comes through, I assume that's good enough.
There was a problem hiding this comment.
Oh, good point. Thanks for clarifying.
No description provided.