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

[14.0][FIX] endpoint_route_handler: Use dedicated cursor for registry #37

Merged

Conversation

grindtildeath
Copy link
Contributor

@grindtildeath grindtildeath commented Mar 4, 2024

When the request cursor is used to instantiate the EndpointRegistry in the call to routing_map, the READ REPEATABLE isolation level will ensure that any value read from the DB afterwards, will be the same than when the first SELECT is executed.

This is breaking the oauth flow as the oauth token that is written at the beginning of the oauth process cannot be read by the cursor computing the session token, which will read an old value. Therefore when the session security check is performed, the session token is outdated as the new session token is computed using an up to date cursor.

By using a dedicated cursor to instantiate the EndpointRegistry, we ensure no read is performed on the database using the request cursor which will in turn use the updated value of the oauth token to compute the session token, and the security check will not fail.

@OCA-git-bot
Copy link
Contributor

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

@grindtildeath grindtildeath force-pushed the 14.0-fix-endpoint_route_handler_cursor branch from 2a4cc86 to 2e89bc3 Compare March 4, 2024 17:43
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

Thanks for tracking down this issue :)

endpoint_route_handler/models/ir_http.py Outdated Show resolved Hide resolved
endpoint_route_handler/models/ir_http.py Show resolved Hide resolved
@simahawk
Copy link
Contributor

simahawk commented Mar 5, 2024

One last bit: can you add the explanation as a comment?

@simahawk
Copy link
Contributor

simahawk commented Mar 5, 2024

I'll apply the changes

@simahawk simahawk force-pushed the 14.0-fix-endpoint_route_handler_cursor branch from 2e89bc3 to 3b91b2f Compare March 5, 2024 10:36
@gurneyalex gurneyalex added this to the 14.0 milestone Apr 26, 2024
When the request cursor is used to instantiate the EndpointRegistry
in the call to routing_map, the READ REPEATABLE isolation level
will ensure that any value read from the DB afterwards, will be the
same than when the first SELECT is executed.

This is breaking the oauth flow as the oauth token that is written
at the beggining of the oauth process cannot be read by the cursor
computing the session token, which will read an old value. Therefore
when the session security check is performed, the session token
is outdated as the new session token is computed using an up to date
cursor.

By using a dedicated cursor to instantiate the EndpointRegistry, we
ensure no read is performed on the database using the request cursor
which will in turn use the updated value of the oauth token to compute
the session token, and the security check will not fail.
@simahawk simahawk force-pushed the 14.0-fix-endpoint_route_handler_cursor branch from 3b91b2f to 647787d Compare July 4, 2024 07:07
@simahawk simahawk self-requested a review July 4, 2024 07:10
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

Live on prod since 4 months.

@simahawk
Copy link
Contributor

simahawk commented Jul 4, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-37-by-simahawk-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1edc2fc into OCA:14.0 Jul 4, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 1aaec8f. 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.

4 participants