Store user session in browser #219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feedmeapples
force-pushed
the
auth
branch
3 times, most recently
from
5a5adf8 to
5a9eac5
Compare
feedmeapples
force-pushed
the
auth
branch
3 times, most recently
from
cf19227 to
230d5b4
Compare
feedmeapples
requested review from
Alex-Tideman,
GiantRobots,
laurakwhit,
rossedfort and
sergeybykov
rossedfort
approved these changes
Sep 23, 2022
GiantRobots
reviewed
Sep 23, 2022
| } else { | ||
| fmt.Println("Using cookie session store") | ||
| } | ||
|
|
GiantRobots
reviewed
Sep 23, 2022
| metaT := "<meta name=\"%s\" content=\"%s\" />" | ||
| assert.Contains(t, string(html), fmt.Sprintf(metaT, "return-url", "http://localhost:8080/namespaces/default")) | ||
| assert.Contains(t, string(html), fmt.Sprintf(metaT, "public-path", "/custom-path")) | ||
| } |
GiantRobots
reviewed
Sep 23, 2022
server/auth/auth_test.html
Outdated
| <meta name="return-url" content="{{.ReturnUrl}}" /> | ||
| <meta name="public-path" content="{{.PublicPath}}" /> | ||
| </head> | ||
| </html> |
There was a problem hiding this comment.
Just thinking out loud headers may be a good way to send this data back in the callback instead of the meta of the html document.
There was a problem hiding this comment.
i want to revisit this particular piece (sending access-token to client) because when returning an html we are limitied to only the current domain and can't serve as a true API only server when it comes to auth endpoint
So instead of sending html at all, get back setting cookie items with users data, except for also breaking down the user object into 4kb cookie items to not exceed the size limit
GiantRobots
approved these changes
Sep 23, 2022
feedmeapples
force-pushed
the
auth
branch
5 times, most recently
from
b5fc1da to
fc2069f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was changed
Moved user session to be stored client side
Why?
resolve #216
Allows to scale UI horizontally by making ui-server stateless
Checklist
Closes
How was this tested:
E2E: Verified that a user can login
Manual: Verified login and logout flows. Verified that API requests contain
authorizationandauthorization-extrasheaders. Verified that codec requests containauthorizationheader