-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Ldap2Azure #117
base: main
Are you sure you want to change the base?
Add Ldap2Azure #117
Conversation
422926c
to
9b2e7a1
Compare
Comments die op MAJOR PR staan zijn hier niet in verwerkt. |
Je geeft me wel veel werk zo :p |
Mwa veel comments, maar weinig werk om op te lossen. |
Voor nu is het meeste muv |
Ik zie geen veranderingen in de PR staan? |
Ik wel: |
8a90fdd
to
66788a4
Compare
08fc303
to
6b805c4
Compare
$dbUser = null; | ||
try { | ||
// We first check if we already have this user in our database | ||
$dbUser = \App\Models\User::findOrFail($user->description); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Betekent dit dat het lidnummer in 'description' staat? Ik zag ergens anders in de code ook 'employeeNumber', waarom hier description en in LoginController.php employeeNumber?
$this->surname = $content['surname']; | ||
$this->givenName = $content['givenName']; | ||
$this->userPrincipalName = $content['userPrincipalName']; | ||
$this->description = Arr::has($content,$descriptionProperty) ? Arr::join( $content[$descriptionProperty] , ',' ) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beetje dezelfde vraag als in Application.php: wat zit eigenlijk in deze property? Vooral vergeleken met de employeeNumber
|
||
$user = $this->getLdapUserBy('description', $lidnummer); | ||
$azureAppInfo = new azureAppInfo; | ||
$user = $azureAppInfo->getAzureUserBy('description', $lidnummer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zelfde vraag met description/employeeNumber
@Kipjr Ik heb alles gereviewed! Eigenlijk maar 1 dingetje waar ik vragen over heb (de comments hierboven) maar als dat opgelost is dan zal ik hem goedkeuren :) |
No description provided.