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_oauth_multi_token: Migration to 18.0 #743

Open
wants to merge 20 commits into
base: 18.0
Choose a base branch
from

Conversation

kobros-tech
Copy link

Based on PR 742 which is just improvement of PR 694
#694 => #742 [17.0][MIG]

Florent de Labarre and others added 20 commits December 31, 2024 12:41
Allow multiple oauth login at the same time.
* cleanup, improve, docstrings
* add tests
Nothing special. Just making linters happy and splitting the readme.

@Tecnativa TT25619
Otherwise lookup is slow when there are many users.
Otherwise you can't delete a user.
This is cosmetic only, because this field is not used
when auth_oauth_multi_token is installed.
Currently translated at 100.0% (15 of 15 strings)

Translation: server-auth-16.0/server-auth-16.0-auth_oauth_multi_token
Translate-URL: https://translation.odoo-community.org/projects/server-auth-16-0/server-auth-16-0-auth_oauth_multi_token/it/
Copy link

@dnplkndll dnplkndll left a comment

Choose a reason for hiding this comment

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

looks good functional test, oidc aws cognito

@kobros-tech
Copy link
Author

@sbidoul could you review this PR and 17.0 one too?

)
# In version 18.0 AccessDenied exception is raising ValueError exception
except ValueError as e:
self.assertEqual(str(e), "Unknown token version")
Copy link
Member

Choose a reason for hiding this comment

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

This change to the test looks suspicious to me. It probably does not test what it was meant to test anymore.

Copy link
Author

Choose a reason for hiding this comment

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

not at all, in version 17.0
there is one single exception raised, the one you mean!

In version 18.0, the exception you mean is raising another exception which is the source one.

I handle it and then I trace the log message for the one you mean.

If you like to update the test case all right show me your opinion.

Copy link

@dnplkndll dnplkndll Jan 17, 2025

Choose a reason for hiding this comment

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

could I suggest:
a8955ff
prevent attempt to create versus refuse. or did we want to change the expects to be a new user?
'Exception: Unknown token version'
is thrown in the https://github.com/odoo/odoo/blob/18.0/addons/auth_oauth/models/res_users.py#L111 I believe.

@kobros-tech kobros-tech requested a review from sbidoul January 5, 2025 15:58
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.

8 participants