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

Delay requests for invalid session #2073

Merged
merged 4 commits into from Apr 18, 2016
Merged

Conversation

@dennisoelkers
Copy link
Member

@dennisoelkers dennisoelkers commented Apr 13, 2016

This PR adds a simple check during initialisation of the SessionStore which validates that a persisted session id is still valid. If it is not, then all requests coming in will be delayed while listening for the login event, just like in the case where no session id exists at all.

This fixes a number of problems related to stores doing initialisation only once and fail when the session id is stale.

Fixes #1891, #2059.

@dennisoelkers dennisoelkers added the web label Apr 13, 2016
@dennisoelkers dennisoelkers added this to the 2.0.0 milestone Apr 13, 2016
@edmundoa edmundoa self-assigned this Apr 13, 2016
@@ -120,6 +119,11 @@ public SessionResponse newSession(@Context ContainerRequestContext requestContex
throw new NotAuthorizedException("Invalid username or password", "Basic realm=\"Graylog Server session\"");
}

@GET
@ApiOperation(value = "Validate an existing session", notes = "Checks the session with the given ID: return OK if session is valid.")

This comment has been minimized.

@edmundoa

edmundoa Apr 14, 2016
Member

Just a bit of nit-picking: the description says it will return an OK if the session is valid, but it actually responds No Content. We could change the method to return an OK or just say it returns a success code.

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 18, 2016
Author Member

👍 this is actually Jersey being more intelligent than me :)

@edmundoa
Copy link
Member

@edmundoa edmundoa commented Apr 14, 2016

Other than the (tiny) remark with the REST documentation, looks good to me.

This also delays the initialization of some stores, preventing errors
where first store initialization fails because of invalid session id,
so the store never loads.

Fixing some linter hints while on the way.

Fixes #2059, #1891.
@dennisoelkers dennisoelkers force-pushed the delay-requests-for-invalid-session branch from 1283922 to 20b3d13 Apr 18, 2016
@edmundoa
Copy link
Member

@edmundoa edmundoa commented Apr 18, 2016

LGTM 👍

@edmundoa edmundoa merged commit 3a1fcc0 into master Apr 18, 2016
4 checks passed
4 checks passed
ci-server-integration Jenkins build graylog2-server-integration-pr 836 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 324 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
@edmundoa edmundoa deleted the delay-requests-for-invalid-session branch Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.