commit 8106b6985620956ce8cfa4126143a95ca87ea976
parent 7ca5bac7c678dff0e9fb33b3cf5824165ca9d3f1
Author: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Thu, 28 Jul 2022 16:43:27 +0200
[feature] add 'state' oauth2 param to /oauth/authorize (#730)
Diffstat:
6 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/internal/api/client/auth/auth.go b/internal/api/client/auth/auth.go
@@ -55,13 +55,14 @@ const (
callbackStateParam = "state"
callbackCodeParam = "code"
- sessionUserID = "userid"
- sessionClientID = "client_id"
- sessionRedirectURI = "redirect_uri"
- sessionForceLogin = "force_login"
- sessionResponseType = "response_type"
- sessionScope = "scope"
- sessionState = "state"
+ sessionUserID = "userid"
+ sessionClientID = "client_id"
+ sessionRedirectURI = "redirect_uri"
+ sessionForceLogin = "force_login"
+ sessionResponseType = "response_type"
+ sessionScope = "scope"
+ sessionInternalState = "internal_state"
+ sessionClientState = "client_state"
)
// Module implements the ClientAPIModule interface for
diff --git a/internal/api/client/auth/authorize.go b/internal/api/client/auth/authorize.go
@@ -189,6 +189,11 @@ func (m *Module) AuthorizePOSTHandler(c *gin.Context) {
errs = append(errs, fmt.Sprintf("key %s was not found in session", sessionScope))
}
+ var clientState string
+ if s, ok := s.Get(sessionClientState).(string); ok {
+ clientState = s
+ }
+
userID, ok := s.Get(sessionUserID).(string)
if !ok {
errs = append(errs, fmt.Sprintf("key %s was not found in session", sessionUserID))
@@ -246,6 +251,10 @@ func (m *Module) AuthorizePOSTHandler(c *gin.Context) {
sessionUserID: {userID},
}
+ if clientState != "" {
+ c.Request.Form.Set("state", clientState)
+ }
+
if err := m.processor.OAuthHandleAuthorizeRequest(c.Writer, c.Request); err != nil {
api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error(), helpfulAdvice), m.processor.InstanceGet)
}
@@ -285,7 +294,8 @@ func saveAuthFormToSession(s sessions.Session, form *model.OAuthAuthorize) gtser
s.Set(sessionClientID, form.ClientID)
s.Set(sessionRedirectURI, form.RedirectURI)
s.Set(sessionScope, form.Scope)
- s.Set(sessionState, uuid.NewString())
+ s.Set(sessionInternalState, uuid.NewString())
+ s.Set(sessionClientState, form.State)
if err := s.Save(); err != nil {
err := fmt.Errorf("error saving form values onto session: %s", err)
diff --git a/internal/api/client/auth/callback.go b/internal/api/client/auth/callback.go
@@ -45,26 +45,26 @@ func (m *Module) CallbackGETHandler(c *gin.Context) {
// check the query vs session state parameter to mitigate csrf
// https://auth0.com/docs/secure/attack-protection/state-parameters
- state := c.Query(callbackStateParam)
- if state == "" {
+ returnedInternalState := c.Query(callbackStateParam)
+ if returnedInternalState == "" {
m.clearSession(s)
err := fmt.Errorf("%s parameter not found on callback query", callbackStateParam)
api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
return
}
- savedStateI := s.Get(sessionState)
- savedState, ok := savedStateI.(string)
+ savedInternalStateI := s.Get(sessionInternalState)
+ savedInternalState, ok := savedInternalStateI.(string)
if !ok {
m.clearSession(s)
- err := fmt.Errorf("key %s was not found in session", sessionState)
+ err := fmt.Errorf("key %s was not found in session", sessionInternalState)
api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
return
}
- if state != savedState {
+ if returnedInternalState != savedInternalState {
m.clearSession(s)
- err := errors.New("mismatch between query state and session state")
+ err := errors.New("mismatch between callback state and saved state")
api.ErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGet)
return
}
diff --git a/internal/api/client/auth/signin.go b/internal/api/client/auth/signin.go
@@ -58,16 +58,16 @@ func (m *Module) SignInGETHandler(c *gin.Context) {
// idp provider is in use, so redirect to it
s := sessions.Default(c)
- stateI := s.Get(sessionState)
- state, ok := stateI.(string)
+ internalStateI := s.Get(sessionInternalState)
+ internalState, ok := internalStateI.(string)
if !ok {
m.clearSession(s)
- err := fmt.Errorf("key %s was not found in session", sessionState)
+ err := fmt.Errorf("key %s was not found in session", sessionInternalState)
api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
return
}
- c.Redirect(http.StatusSeeOther, m.idp.AuthCodeURL(state))
+ c.Redirect(http.StatusSeeOther, m.idp.AuthCodeURL(internalState))
}
// SignInPOSTHandler should be served at https://example.org/auth/sign_in.
diff --git a/internal/api/client/status/statuscreate_test.go b/internal/api/client/status/statuscreate_test.go
@@ -312,7 +312,7 @@ func (suite *StatusCreateTestSuite) TestAttachNewMediaSuccess() {
ctx.Request = httptest.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost:8080/%s", status.BasePath), nil) // the endpoint we're hitting
ctx.Request.Header.Set("accept", "application/json")
ctx.Request.Form = url.Values{
- "status": {"here's an image attachment"},
+ "status": {"here's an image attachment"},
"media_ids[]": {attachment.ID},
}
suite.statusModule.StatusCreatePOSTHandler(ctx)
diff --git a/internal/api/model/oauth.go b/internal/api/model/oauth.go
@@ -33,4 +33,8 @@ type OAuthAuthorize struct {
// List of requested OAuth scopes, separated by spaces (or by pluses, if using query parameters).
// Must be a subset of scopes declared during app registration. If not provided, defaults to read.
Scope string `form:"scope" json:"scope"`
+ // State is used by the application to store request-specific data and/or prevent CSRF attacks.
+ // The authorization server must return the unmodified state value back to the application.
+ // See https://www.oauth.com/oauth2-servers/authorization/the-authorization-request/
+ State string `form:"state" json:"state"`
}
