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 cookie settings to use plugin AuthFlags #1424

Merged
merged 1 commit into from Dec 21, 2018

Conversation

Projects
None yet
3 participants
@vboctor
Copy link
Member

commented Dec 20, 2018

Fixes #25099

@vboctor vboctor self-assigned this Dec 20, 2018

@dregad

dregad approved these changes Dec 20, 2018

@@ -190,10 +190,11 @@ function auth_anonymous_account() {
/**
* Get the auth cookie expiry time.
* @param boolean $p_perm_login Use permanent login.
* @param int $p_user_id The user id to get session expiry for.

This comment has been minimized.

Copy link
@dregad

dregad Dec 20, 2018

Member

add defaults to current user

This comment has been minimized.

Copy link
@vboctor

vboctor Dec 20, 2018

Author Member

@dregad This method is only called from one place which I changed. I defaulted it to null just in case it is being called from plugins. The null value means user not specified and hence will use default AuthFlags since there is no authenticated / known user at this time (this flow happens before auth cookie is set).

I can remove it or clarify in the comment.

This comment has been minimized.

Copy link
@dregad

dregad Dec 20, 2018

Member

I can [...] clarify in the comment.

That's what I meant. Thanks

@@ -190,10 +190,11 @@ function auth_anonymous_account() {
/**
* Get the auth cookie expiry time.
* @param boolean $p_perm_login Use permanent login.
* @param int $p_user_id The user id to get session expiry for.

This comment has been minimized.

Copy link
@atrol

atrol Dec 21, 2018

Member

Concerning @param int vs. integer.
We don't have clear rules for that, but just one line below and most of the timeinteger is used.

@atrol

atrol approved these changes Dec 21, 2018

@vboctor vboctor force-pushed the vboctor:issue25099_auth_flags_cookie_fix branch from cb25b85 to 7a5aee9 Dec 21, 2018

@vboctor

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2018

I removed the defaulting to null to avoid confusion and because it is not needed. I have also applied @atrol's comment about the integer vs. int, and I also swapped the order of the parameters.

@vboctor vboctor merged commit e16586b into mantisbt:master Dec 21, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.