added fetching groups of user if the scope includes 'api', extended and refactored tests - #14
Open
zergar wants to merge 3 commits into
Open
added fetching groups of user if the scope includes 'api', extended and refactored tests#14zergar wants to merge 3 commits into
zergar wants to merge 3 commits into
Conversation
…nd refactored tests This commit includes changes that extend the profile-object returned by this application to include the groups the user has access to. This is done by means of a nested callback which is executed if the scope includes 'api'. The callback extends the profile object by the `groups`-property, which value is an array containing the group names. To test the behaviour of the newly added code, the strategy.profile.spec.js was extended with more tests. It was necessary to wrap the _oauth.get-method inside a closure to generate different behaviour, as this class mocks the behaviour of that method. The oAuthGetReplacementClosure now accepts a config object to e.g. return a malformed response to the groups request, but a correct one to the profile-request. The existing tests were adapted to use the newly introduced method. Signed-off-by: Gereon Dusella <git@gereondusella.de>
Signed-off-by: Gereon Dusella <git@gereondusella.de>
the strategy.spec.js missed the query parameter which limits the administrator from being included in all groups. Signed-off-by: Gereon Dusella <git@gereondusella.de>
|
Hello @fh1ch, how are you ? The passport-gitlab2 module, is it still maintained ? Do you think, you can merge this MR to the module ? I'm looking for a way to filter logins based on the group. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This commit includes changes that extend the profile-object returned by this application to include the groups the user has access to. This is done by means of a nested callback which is executed if the scope includes 'api'. The callback extends the profile object by the
groups-property, which value is an array containing the group names.To test the behaviour of the newly added code, the strategy.profile.spec.js was extended with more tests. It was necessary to wrap the _oauth.get-method inside a closure to generate different behaviour, as this class mocks the behaviour of that method. The oAuthGetReplacementClosure now accepts a config object to e.g. return a malformed response to the groups request, but a correct one to the profile-request. The existing tests were adapted to use the newly introduced method.
Further information
This feature is useful if an existing gitlab-deployment shall be used for login, but not every user should have access to the application. To my knowledge, there exists no standardized way for writing groups to the user profile. Therefore, this commit uses the most basic approach of just adding the groups as an array of strings.