-
Notifications
You must be signed in to change notification settings - Fork 273
chore(auth): Inline ID token body parsing in session login snippets #777
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
base: dev
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,6 @@ import ( | |
| "context" | ||
| "encoding/base64" | ||
| "encoding/json" | ||
| "io/ioutil" | ||
| "log" | ||
| "net/http" | ||
| "time" | ||
|
|
@@ -631,8 +630,10 @@ func loginHandler(client *auth.Client) http.HandlerFunc { | |
| return func(w http.ResponseWriter, r *http.Request) { | ||
| // Get the ID token sent by the client | ||
| defer r.Body.Close() | ||
| idToken, err := getIDTokenFromBody(r) | ||
| if err != nil { | ||
| var requestBody struct { | ||
| IDToken string `json:"idToken"` | ||
| } | ||
| if err := json.NewDecoder(r.Body).Decode(&requestBody); err != nil { | ||
| http.Error(w, err.Error(), http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
@@ -644,7 +645,7 @@ func loginHandler(client *auth.Client) http.HandlerFunc { | |
| // The session cookie will have the same claims as the ID token. | ||
| // To only allow session cookie setting on recent sign-in, auth_time in ID token | ||
| // can be checked to ensure user was recently signed in before creating a session cookie. | ||
| cookie, err := client.SessionCookie(r.Context(), idToken, expiresIn) | ||
| cookie, err := client.SessionCookie(r.Context(), requestBody.IDToken, expiresIn) | ||
| if err != nil { | ||
| http.Error(w, "Failed to create a session cookie", http.StatusInternalServerError) | ||
| return | ||
|
|
@@ -668,13 +669,15 @@ func loginWithAuthTimeCheckHandler(client *auth.Client) http.HandlerFunc { | |
| return func(w http.ResponseWriter, r *http.Request) { | ||
| // Get the ID token sent by the client | ||
| defer r.Body.Close() | ||
| idToken, err := getIDTokenFromBody(r) | ||
| if err != nil { | ||
| var requestBody struct { | ||
| IDToken string `json:"idToken"` | ||
| } | ||
| if err := json.NewDecoder(r.Body).Decode(&requestBody); err != nil { | ||
| http.Error(w, err.Error(), http.StatusBadRequest) | ||
| return | ||
| } | ||
|
Comment on lines
+672
to
678
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To prevent potential Denial of Service (DoS) attacks via resource exhaustion, it is highly recommended to limit the request body size using r.Body = http.MaxBytesReader(w, r.Body, 1048576)
var requestBody struct {
IDToken string `json:"idToken"`
}
if err := json.NewDecoder(r.Body).Decode(&requestBody); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
if requestBody.IDToken == "" {
http.Error(w, "idToken is required", http.StatusBadRequest)
return
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
|
|
||
| decoded, err := client.VerifyIDToken(r.Context(), idToken) | ||
| decoded, err := client.VerifyIDToken(r.Context(), requestBody.IDToken) | ||
| if err != nil { | ||
| http.Error(w, "Invalid ID token", http.StatusUnauthorized) | ||
| return | ||
|
|
@@ -686,7 +689,7 @@ func loginWithAuthTimeCheckHandler(client *auth.Client) http.HandlerFunc { | |
| } | ||
|
|
||
| expiresIn := time.Hour * 24 * 5 | ||
| cookie, err := client.SessionCookie(r.Context(), idToken, expiresIn) | ||
| cookie, err := client.SessionCookie(r.Context(), requestBody.IDToken, expiresIn) | ||
| if err != nil { | ||
| http.Error(w, "Failed to create a session cookie", http.StatusInternalServerError) | ||
| return | ||
|
|
@@ -808,19 +811,6 @@ func sessionLogoutHandlerWithRevocation(client *auth.Client) http.HandlerFunc { | |
| // [END session_clear_and_revoke] | ||
| } | ||
|
|
||
| func getIDTokenFromBody(r *http.Request) (string, error) { | ||
| b, err := ioutil.ReadAll(r.Body) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| var parsedBody struct { | ||
| IDToken string `json:"idToken"` | ||
| } | ||
| err = json.Unmarshal(b, &parsedBody) | ||
| return parsedBody.IDToken, err | ||
| } | ||
|
|
||
| func newActionCodeSettings() *auth.ActionCodeSettings { | ||
| // [START init_action_code_settings] | ||
| actionCodeSettings := &auth.ActionCodeSettings{ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent potential Denial of Service (DoS) attacks via resource exhaustion, it is highly recommended to limit the request body size using
http.MaxBytesReaderbefore decoding. Additionally, we should validate that theidTokenis not empty to avoid unnecessary downstream processing and return a400 Bad Requestinstead of a500 Internal Server Errorwhen the token is missing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these are just for snippets I think this is a bit much. These types of optimizations should be left to the developer.