Auth frontend #425
Auth frontend #425
Conversation
Signed-off-by: Maxim Sukharev <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
| w.Write(b) | ||
| } | ||
|
|
||
| func errorJSON(w http.ResponseWriter, r *http.Request, code int, errors ...error) { |
There was a problem hiding this comment.
Why is this defined, but then http.Error is used?
There was a problem hiding this comment.
just for testing. As I mentioned in PR description: "This PR doesn't cover proper serialization of responses"
Signed-off-by: Maxim Sukharev <[email protected]>
| AllowedOrigins: []string{"*"}, | ||
| // TODO: make it customizable | ||
| // we can't pass "*" because it's incompatible with "credentials: include" request | ||
| AllowedOrigins: []string{"http://127.0.0.1:3000"}, |
There was a problem hiding this comment.
it's default dev port for create-react-app.
There was a problem hiding this comment.
OK thanks. Then maybe we could add a comment saying why this is here, it's not so obvious that this is required for development.
|
I have a question. Is it unavoidable to have client-side routing? It feels it might add complexity down the road. Are there any problems with leaving the current |
|
Router is required by the issue description written by you. Do you suggest to add more |
Signed-off-by: Maxim Sukharev <[email protected]>
|
I'm not suggesting anything, and I'm not against using client-side routing. I was asking why is it needed. Going through the alternatives in my mind I see I was not aware of the need for client-side routing because I was thinking on the way we defined the SSO OKR design doc, with separate endpoints for login and logout. Which in this case will probably complicate things too much, and it makes sense to keep it as a single page app with all the token logic in the same place. On the other hand, if the goal of the frontend was to keep it as simple as possible, we could also skip routing and have the frontend work as the Anyway I see the router already implemented in #433 so I guess there is no point in discussing if we need to implement it or not. |
|
Link to issue: #399 |
|
I don't mind dropping routing PR if everybody agrees to change original requirements and serve everything on |
|
LGTM, we can discuss the need for a router in #433. |
|
I apply 7 days rule here and merge. |
I changed the token response to be compatible with our regular response format.
This PR doesn't cover proper serialization of responses, there are only a few new lines of code to create the same data shape for frontend. We can copy-past it from gitbase-web later or create some improved version.
I also introduced new
/api/meendpoint. I think it belongs to auth service.API js code is a little bit different (simpler) than what we did in other projects.
Apparently, we don't need all that checks if we follow correct types.
Implemented in separate PR:
TODO: