8000 Issue 1347 - Gitlab connector should not require the api scope. by gypsydiver · Pull Request #1351 · dexidp/dex · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Issue 1347 - Gitlab connector should not require the api scope. #1351

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 20, 2018
Merged

Issue 1347 - Gitlab connector should not require the api scope. #1351

merged 1 commit into from
Nov 20, 2018

Conversation

gypsydiver
Copy link
Contributor

@gypsydiver gypsydiver changed the title Issue 1347 Gitlab connector should not require the api scope. Issue 1347 - Gitlab connector should not require the api scope. Nov 20, 2018
srenatus
srenatus approved these changes Nov 20, 2018
View reviewed changes
Copy link
Contributor
@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Code-wise, this looks good, and it's also gotten a bit simpler 🎉 Thanks for your contribution.

I'm not completely sure I understand the change from a current user's perspective: if you've used the gitlab connector before this, can you use it after this in just the same way -- or is there additional steps required on the gitlab side regarding the changed scopes? (Or is it just broken right now?)

@srenatus
Copy link
Contributor

Update -- I've read #1347 again. So, this is meant to use the simpler way, with a less permissive scope? If so, 👍

Still interested in the user-facing changes, if there are any.

@gypsydiver
Copy link
Contributor Author

Changes here would break applications that don't have the openid scope enabled and require to pull groups from Gitlab. Admins need to at least enable it in the Gitlab Application settings for groups to work; and ideally disable the api (though not required).

@srenatus
Copy link
Contributor

👍 sounds great. Do you happen to know when that feature as introduced into Gitlab?

@gypsydiver
Copy link
Contributor Author

Gitlab CE Version 9.0.0, 2017-03.

@srenatus
Copy link
Contributor

Thanks for looking that up. Judging from what, it's been out for more than 18 months, and reducing the application scope there is a bit of a significant security improvement, I suppose. I'll merge this.

⏩ Let's make sure to include this information in the next release notes, so nobody is in for a bad surprise.

@srenatus srenatus merged commit 84ea412 into dexidp:master Nov 20, 2018
@srenatus srenatus mentioned this pull request Apr 29, 2019
Closed
mmrath pushed a commit to mmrath/dex that referenced this pull request Sep 2, 2019
@srenatus
Merge pull request dexidp#1351 from CognotektGmbH/gypsydiver/1347-pr-…
cbd9092 
…gitlab-groups

Gitlab connector should not require the api scope.

Fixes dexidp#1347.
@nabokihms nabokihms mentioned this pull request Apr 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srenatus srenatus srenatus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gypsydiver @srenatus
0