Skip to content

chore(auth): Inline ID token body parsing in session login snippets - #777

Open
jonathanedey wants to merge 1 commit into
devfrom
je-snip
Open

chore(auth): Inline ID token body parsing in session login snippets#777
jonathanedey wants to merge 1 commit into
devfrom
je-snip

Conversation

@jonathanedey

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors snippets/auth.go by removing the helper function getIDTokenFromBody and the unused io/ioutil import, opting instead to decode the JSON request body inline using json.NewDecoder within loginHandler and loginWithAuthTimeCheckHandler. The review feedback suggests improving security and robustness by limiting the request body size using http.MaxBytesReader to prevent potential Denial of Service (DoS) attacks, and validating that the idToken is not empty before processing.

Comment thread snippets/auth.go
Comment on lines +633 to 639
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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential Denial of Service (DoS) attacks via resource exhaustion, it is highly recommended to limit the request body size using http.MaxBytesReader before decoding. Additionally, we should validate that the idToken is not empty to avoid unnecessary downstream processing and return a 400 Bad Request instead of a 500 Internal Server Error when the token is missing.

Suggested change
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
}
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
}

Comment thread snippets/auth.go
Comment on lines +672 to 678
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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential Denial of Service (DoS) attacks via resource exhaustion, it is highly recommended to limit the request body size using http.MaxBytesReader before decoding. Additionally, we should validate that the idToken is not empty to avoid unnecessary downstream processing and return a 400 Bad Request instead of a 401 Unauthorized when the token is missing.

		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
		}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant