Skip to content
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

Fix account creation with the registration app #144

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Fix account creation with the registration app #144

merged 1 commit into from
Nov 12, 2020

Conversation

AlbanBedel
Copy link
Contributor

When an account is created from the registration app there is no user session. This lead to an Error in createUser() as $adminUser is used while null, but only Exceptions are cought there.

Add a check that throw an Exception when $adminUser is null to fallback in the code path that handle the case where the admin user is not from the LDAP directory.

The documentation should perhaps also mention that using the "admin must be from LDAP" feature is not compatible with the registration app.

@AlbanBedel
Copy link
Contributor Author

Is there really no interest in having this bug fixed?

@AlbanBedel
Copy link
Contributor Author

Can this please be looked at? I really like to stop having to use my own fork and have this bug fixed in the main version.

Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay!

lib/LDAPUserManager.php Outdated Show resolved Hide resolved
@AlbanBedel
Copy link
Contributor Author

All points from the review have been fixed, can this be merged?

@blizzz
Copy link
Member

blizzz commented Nov 4, 2020

Same as with the other PR. Mind a rebase?

When an account is created from the registration app there is no user
session. This lead to an Error in createUser() as $adminUser is used
while null, but only Exceptions are cought there.

Add a check that throw an Exception when $adminUser is null to
fallback in the code path that handle the case where the admin user is
not from the LDAP directory.

Signed-off-by: Alban Bedel <[email protected]>
@AlbanBedel
Copy link
Contributor Author

Rebase done

@blizzz blizzz merged commit 14d7936 into nextcloud:master Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants