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

[18.0][MIG] auth_oidc: Migration to 18.0 #705

Merged
merged 40 commits into from
Oct 23, 2024

Conversation

ap-wtioit
Copy link
Contributor

info @wt-io-it

andreschenkels and others added 30 commits October 9, 2024 12:35
update manifest, add README, update requirements.txt
Add some install instructions and configuration instructions
for keycloak.
Avoid replacing the access token by the id token.
This may cause confusion.
Copy a little piece of code from auth_oauth() method,
to make the code easier to follow, and prepare for
implementing the authorization code flow.
This is not a recommended scenario, but this prepares
the code for using PKCE
create an upstream merge request after
OCA#393 is merged
@ap-wtioit
Copy link
Contributor Author

ap-wtioit commented Oct 9, 2024

/ocabot migration auth_oidc

@OCA-git-bot
Copy link
Contributor

Sorry @ap-wtioit you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@ap-wtioit ap-wtioit mentioned this pull request Oct 9, 2024
13 tasks
@sbidoul
Copy link
Member

sbidoul commented Oct 9, 2024

/ocabot migration auth_oidc

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Oct 9, 2024
@sbidoul
Copy link
Member

sbidoul commented Oct 9, 2024

I compared with 17, and it's identical.

Compared to 16, there are a couple of PR's that landed there that were never ported to 17 (#573, #650) - not saying you need to do it, just a note.

Copy link
Contributor

@hparfr hparfr left a comment

Choose a reason for hiding this comment

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

LGTM (not tested yet)

@sbidoul
Copy link
Member

sbidoul commented Oct 23, 2024

Happy to merge this as soon as someone confirms it has been functionally tested.

Copy link
Member

@wtaferner wtaferner left a comment

Choose a reason for hiding this comment

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

Functionally tested within our own OAuth Provider Infrastructure and logged in successfully on the target test instance with OpenID

@sbidoul
Copy link
Member

sbidoul commented Oct 23, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-705-by-sbidoul-bump-nobump, awaiting test results.

@hparfr
Copy link
Contributor

hparfr commented Oct 23, 2024

Tested as well with keyloack; works as expected.

@OCA-git-bot OCA-git-bot merged commit 06c1bed into OCA:18.0 Oct 23, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.