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

Prevent useless session validation when no id or username is present. #3634

Merged
merged 1 commit into from Mar 17, 2017

Conversation

Projects
None yet
2 participants
@dennisoelkers
Member

dennisoelkers commented Mar 17, 2017

This change checks for the presence of a session id or username before
validating the session. This has two benefits:

  1. A round trip to the server is saved upon initial login, showing the
    login dialog much faster.
  2. A race condition is prevented in automated testing when the session
    id is added to localStorage between two page visits. The code does not
    check if a validation is currently in progress before triggering a new
    one, which leads to inconsistent session propagation.

Also needs to be merged into 2.2.

Prevent useless session validation when no id or username is present.
This change checks for the presence of a session id or username before
validating the session. This has two benefits:

  1) A round trip to the server is saved upon initial login, showing the
login dialog much faster.
  2) A race condition is prevented in automated testing when the session
id is added to localStorage between two page visits. The code does not
check if a validation is currently in progress before triggering a new
one, which leads to inconsistent session propagation.
@joschi

joschi approved these changes Mar 17, 2017

@joschi joschi self-assigned this Mar 17, 2017

@joschi joschi merged commit c3983db into master Mar 17, 2017

4 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 1460 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@joschi joschi deleted the prevent-useless-validation branch Mar 17, 2017

@joschi

This comment has been minimized.

Contributor

joschi commented Mar 17, 2017

Since it's not a bugfix, I wouldn't merge it into the 2.2 branch.

bernd added a commit that referenced this pull request Jul 4, 2017

Revert "Prevent useless session validation when no id or username is …
…present. (#3634)"

This reverts commit c3983db.

The SSO plugin relies on validation of the session to be able to skip the
login form and log in the user automatically.

Fixes #3948

dennisoelkers added a commit that referenced this pull request Jul 4, 2017

Revert "Prevent useless session validation when no id or username is …
…present. (#3634)" (#3973)

This reverts commit c3983db.

The SSO plugin relies on validation of the session to be able to skip the
login form and log in the user automatically.

Fixes #3948

dennisoelkers added a commit that referenced this pull request Jul 28, 2017

Only removing session in localStorage when set.
Before this change, whenever a session validation attempt failed, the
session data in localStorage was removed. This was leading to a race
condition for automated browser testing, when validation took longer
than visiting the page for the first time to put session data in
localStorage, which was immediately removed by the validation promise
handler.

After this change, session data in localStorage is removed only if
present.

Refs #3634, #3948, #3973.

bernd added a commit that referenced this pull request Jul 28, 2017

Only removing session in localStorage when set. (#4041)
Before this change, whenever a session validation attempt failed, the
session data in localStorage was removed. This was leading to a race
condition for automated browser testing, when validation took longer
than visiting the page for the first time to put session data in
localStorage, which was immediately removed by the validation promise
handler.

After this change, session data in localStorage is removed only if
present.

Refs #3634, #3948, #3973.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment