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

[FIX] auth_session_timeout: add /longpolling/im_status as ignored url #689

Merged

Conversation

rolandojduartem
Copy link

@rolandojduartem rolandojduartem commented Sep 13, 2024

auth_session_timeout [1] uses the time of the modification of the session file to calculate the time of inactivity, but any edition in the session file will change the time of modification of the file and cause inexact time of inactivity.

The controller /longpolling/im_status is called several times (every 50 seconds) [2][3], so it is required taking it into account to avoid modifying the time of modification of the session file, so it is required adding /longpolling/im_status as a ignored url [4][5].

References:

Screenshot from 2024-09-11 13-02-20

@rolandojduartem
Copy link
Author

Hi, @hugho-ad, @keylor2906, @imanie383, @luisg123v and @moylop260, please, could you review this PR?

This is related to T#84550

@rolandojduartem rolandojduartem force-pushed the 15.0-auth_session_timeout-fix_timeout-rolando branch from 547fb94 to a97224e Compare September 13, 2024 01:25
Copy link
Contributor

@moylop260 moylop260 left a comment

Choose a reason for hiding this comment

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

👍

env = api.Environment(cr, SUPERUSER_ID, {})
new_url = "/longpolling/im_status"
ignored_path_key = "inactive_session_time_out_ignored_url"
param = env["ir.config_parameter"].sudo()
Copy link
Contributor

Choose a reason for hiding this comment

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

sudo not required as already running as superuser

auth_session_timeout [1] uses the time of the modification of the
session file to calculate the time of inactivity, but any edition in the
session file will change the time of modification of the file and cause
inexact time of inactivity.

The controller /longpolling/im_status is called several times (every 50
seconds) [2][3], so it is required taking it into account to avoid
modifying the time of modification of the session file, so it is
required adding /longpolling/im_status as a ignored url [4][5].

References:
- [1] https://github.com/Oca/server-auth/blob/1db10f29/auth_session_timeout/models/res_users.py#L73
- [2] https://github.com/odoo/odoo/blob/e5b67f9c/addons/mail/static/src/models/partner/partner.js#L395
- [3] https://github.com/odoo/odoo/blob/e5b67f9c/addons/mail/static/src/models/partner/partner.js#L407
- [4] https://github.com/Oca/server-auth/blob/1db10f29/auth_session_timeout/data/ir_config_parameter_data.xml#L12
- [5] https://github.com/Oca/server-auth/blob/1db10f29/auth_session_timeout/models/res_users.py#L93
@rolandojduartem rolandojduartem force-pushed the 15.0-auth_session_timeout-fix_timeout-rolando branch from a97224e to 17ae4f6 Compare September 13, 2024 19:49
@rolandojduartem
Copy link
Author

Hi, @luisg123v, I applied your suggestion, could you review it?

@rolandojduartem
Copy link
Author

Hi, @moylop260, could you merge it?

@moylop260
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-689-by-moylop260-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1fbe170 into OCA:15.0 Sep 13, 2024
8 checks passed
@OCA-git-bot
Copy link
Contributor

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

@luisg123v
Copy link
Contributor

@moylop260 this fits better as nobump because @rolandojduartem already increased version.

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.

5 participants