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

Add bitbucket support #3

Closed
wants to merge 20 commits into from
Closed

Conversation

edtan
Copy link

@edtan edtan commented Sep 16, 2018

This adds a Dex connector for Bitbucket Cloud. It supports mapping a Bitbucket user's teams to the groups claim. Most of the code was based off of the existing Github and Microsoft connectors.

One thing that I'm unsure about is revoked tokens. First, I can successfully log into Concourse using the Bitbucket connector. If I then revoke access to Concourse in Bitbucket, I seem to still have access to Concourse as long as I stay logged in.

Should go along with concourse/concourse#2631

ref. concourse/concourse#2567

Joshua Winters and others added 18 commits September 5, 2018 15:38
- Verifies user is part of orgs and spaces for group claims

Signed-off-by: Shash Reddy <[email protected]>
Signed-off-by: Shash Reddy <[email protected]>
…pr/add-oauth-connector', 'origin/pr/add-web-host-url', 'origin/pr/github-teams-no-org-and-slugs', 'origin/pr/gitlab-broken-auth-headers', 'origin/pr/gitlab-include-user-scope-for-groups-inspection', 'origin/pr/oidc-groups-and-tls', 'origin/pr/optional-prometheus-logger', 'origin/pr/postgres-unix-sockets' and 'origin/pr/unset-pg-serializable-transaction-level'
@edtan edtan force-pushed the add-bitbucket-support branch from b48f803 to f6c8545 Compare September 16, 2018 21:31
@aledegano
Copy link

Thank you so much for this PR. It's going to be super-useful for my team!

@marco-m
Copy link

marco-m commented Sep 26, 2018

@pivotal-jwinters would you have time to have a look at this PR ? This is the last thing holding us from switching to 4.x. Thanks!

@jwntrs
Copy link

jwntrs commented Sep 26, 2018

@edtan thanks for adding this! Do you mind adding a few tests before we merge this in?

@marco-m
Copy link

marco-m commented Sep 28, 2018

@edtan This is missing only the tests :-)

@edtan
Copy link
Author

edtan commented Sep 28, 2018

@marco-m Yup, I'll try to finish off the tests some time this weekend. :)

@vito
Copy link
Member

vito commented Sep 28, 2018

A quick note for anyone reviewing this: we shouldn't merge this in to master because our fork has an auto-maintained master branch that's the union of all of our pr/* branches merged over the upstream Dex repo.

Maybe this should just be PR'd to upstream Dex?

@edtan
Copy link
Author

edtan commented Sep 28, 2018

Ah I see. Sure, I'll submit this to upstream Dex this weekend.

@edtan
Copy link
Author

edtan commented Sep 30, 2018

I've now submitted dexidp#1307 for this, as well as added a few tests to this PR.

@marco-m
Copy link

marco-m commented Sep 30, 2018

Thanks @edtan, deeply appreciated!

@marco-m
Copy link

marco-m commented Sep 30, 2018

@vito once dex upstream accepts the PR, how will it reach concourse/dex ?

- Add Bitbucket tests
- Remove test comments
- Update documentation
- Fix golint issue
@edtan edtan force-pushed the add-bitbucket-support branch from 2cf1aa4 to aeb15e1 Compare September 30, 2018 19:54
@vito
Copy link
Member

vito commented Sep 30, 2018

@edtan Thanks for submitting it there (and thanks for doing this in the first place)!

@marco-m Hmm we should update the README.md in our fork to document this process. How about let's just leave this PR open for now and close it once upstream Dex has merged it and we've pulled it in?

Mechanically, the steps are:

git checkout maintenance
./update-merged-branch

...but this currently has to be run by someone with push access. I guess if you want to try it yourself you could just run it and let it fail at the very end when it tries to push. Your local HEAD will be the newly merged (but un-pushed) master.

@marco-m
Copy link

marco-m commented Oct 12, 2018

@vito this PR has just been merged in dex upstream. Who could we ask in the Concourse team to update concourse/dex ?

@vito
Copy link
Member

vito commented Oct 17, 2018

updated our Dex fork; the upstream changes are now in this tag: https://github.com/concourse/dex/releases/tag/merged-20181017103659

thanks again!

@vito vito closed this Oct 17, 2018
@edtan edtan deleted the add-bitbucket-support branch October 17, 2018 17:43
xtremerui pushed a commit that referenced this pull request Sep 17, 2020
Extracted test cases from OAuth2Code flow tests to reuse in device flow

deviceHandler unit tests to test specific device endpoints

Include client secret as an optional parameter for standards compliance

Signed-off-by: justin-slowik <[email protected]>
xtremerui pushed a commit that referenced this pull request Jan 25, 2024
* Add the ability to composite new claims in the OIDC connector,  based on upstream claims

Signed-off-by: Oded Ben-Ozer <[email protected]>
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.

5 participants