-
-
Notifications
You must be signed in to change notification settings - Fork 422
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
[17.0][IMP] auth_oidc: add groups' handling #682
base: 17.0
Are you sure you want to change the base?
Conversation
Hi @sbidoul, |
ae41059
to
96788d5
Compare
the v14 PR is based on my v12 PR which was merged - why didn't you just forward port this? |
@hbrunn thanks for asking! As I'm quite fresh in the Odoo ecosystem, I did not see the v12 PR. Care to share a link? As you can see from the code, my patch works a bit differently; as it appeared that what I needed for group mapping was directly in the access token, there's no usage of the data_endpoint. But I'm also likely not fluent enough in OAuth2 to know if that is really a correct way too. Well; in any case, I'm happy to work towards merging either this or your v12 PR (or a mix of both) for v17. We need @sbidoul 's input, right? |
you find the v12 PR here |
96788d5
to
b360551
Compare
@hbrunn Great. Thanks for the pointer to the v12 PR. I've now understood the code much better, and did a mostly-straightforward port, with just two minor additions as separate commits. Could you perhaps review? |
As the codecov warnings seem critical, I've now added some more tests around the safe_eval call of the expressions. Edit: and now also added some groups' assignment/deassignment checks, pushing the codecov bar above the needed limits. |
a242ae4
to
7587cf0
Compare
This allows groups' handling via a token's attributes as passed by a Keycloak instance serving as IdP.
@sbidoul : I'd be happy to make any necessary changes!