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.

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

[16.0][IMP] auth_oidc: prompt for account on AAD login #573

Merged
merged 1 commit into from
Apr 12, 2024
Merged

[16.0][IMP] auth_oidc: prompt for account on AAD login #573

merged 1 commit into from
Apr 12, 2024

Conversation

CRogos
Copy link
Contributor

@CRogos CRogos commented Nov 14, 2023

Added the possibility to add additional parameters to the auth_link.
When using the AzureAD auth, this gives the ability to set the "prompt" parameter to "select_account" which allowes to display the select account dialog during the login process. This is helpful, when you have multiple AzureAD accounts.

image

@OCA-git-bot
Copy link
Contributor

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

@CRogos CRogos changed the title [IMP] auth_oidc: prompt for account on AAD login [16.0][IMP] auth_oidc: prompt for account on AAD login Nov 14, 2023
@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch 2 times, most recently from e58f7d6 to 035f78e Compare November 14, 2023 13:39
@CRogos CRogos closed this Nov 14, 2023
@CRogos CRogos reopened this Nov 14, 2023
@OCA-git-bot
Copy link
Contributor

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch from 035f78e to a5c32fe Compare November 14, 2023 14:11
sbidoul
sbidoul reviewed Nov 14, 2023
View reviewed changes
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

Nice!

auth_oidc/controllers/main.py Outdated Show resolved Hide resolved
auth_oidc/views/auth_oauth_provider.xml Outdated Show resolved Hide resolved
@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch from a5c32fe to 36930e1 Compare November 14, 2023 14:18
@CRogos CRogos marked this pull request as ready for review November 14, 2023 14:19
@CRogos CRogos marked this pull request as draft November 14, 2023 14:24
@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch 6 times, most recently from 962add3 to eccd2f4 Compare November 14, 2023 15:11
@CRogos CRogos marked this pull request as ready for review November 14, 2023 15:15
@CRogos CRogos requested a review from sbidoul November 14, 2023 15:16
sbidoul
sbidoul approved these changes Nov 16, 2023
View reviewed changes
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

Still a minor ergnonomy issue, otherwise looks good.

auth_oidc/views/auth_oauth_provider.xml Outdated Show resolved Hide resolved
@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch from eccd2f4 to b42317e Compare November 16, 2023 08:09
@sbidoul
Copy link
Member

sbidoul commented Nov 16, 2023

Ah, can you rebase and resolve the conflict on the CONFIGURE readme fragment?
There were rendering issues with the images, and while fixing that I took the opportunity to convert them to markdown.

@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch from b42317e to 92a8a0e Compare November 16, 2023 11:49
lucuswolfius
lucuswolfius approved these changes Feb 21, 2024
View reviewed changes
Copy link

@lucuswolfius lucuswolfius left a comment

Choose a reason for hiding this comment

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

Code lgtm, nice work

@CRogos
Copy link
Contributor Author

CRogos commented Feb 22, 2024

@sbidoul can you merge?

@dreispt
Copy link
Member

dreispt commented Mar 22, 2024

@CRogos Can you resolve the conflict please?

@CRogos CRogos force-pushed the 16.0-imp-auth_oidc-select-account branch from 92a8a0e to 2010a65 Compare March 22, 2024 21:33
@CRogos
Copy link
Contributor Author

CRogos commented Apr 9, 2024

@dreispt it is rebased. can you merge?

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@dreispt
Copy link
Member

dreispt commented Apr 12, 2024

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-573-by-dreispt-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 446c789 into OCA:16.0 Apr 12, 2024
9 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 0c71e80. Thanks a lot for contributing to OCA. ❤️

@CRogos CRogos deleted the 16.0-imp-auth_oidc-select-account branch April 12, 2024 11:52
@sbidoul
Copy link
Member

sbidoul commented Oct 9, 2024

@CRogos do you plan to port this to 17?

@sbidoul sbidoul mentioned this pull request Oct 9, 2024
Merged
@CRogos CRogos mentioned this pull request Oct 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucuswolfius lucuswolfius lucuswolfius approved these changes

@sbidoul sbidoul sbidoul approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@CRogos @OCA-git-bot @sbidoul @dreispt @lucuswolfius