Skip to content

Commit

Permalink
feat: update oauth1.a flow (#1382)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?
* Previously, we needed to check if the request token generated matches
the oauth token returned in the callback to prevent replay attacks.
* However, this is mitigated by verifying the `state` jwt query param on
the callback.
* The [oauth1.a spec](https://oauth.net/core/1.0a/#auth_step1) also
doesn't mention that the consumer needs to verify that the request token
matches the oauth token returned in the callback
* This also fixes issues where the `_gotrue_session` cookie is not being
sent on the callback, which results in "session could not be found for
this request" error.
  • Loading branch information
kangmingtay authored Jan 19, 2024
1 parent 9ca6970 commit 4f39d2e
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 72 deletions.
3 changes: 0 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ require (
github.com/gobwas/glob v0.2.3
github.com/gofrs/uuid v4.3.1+incompatible
github.com/golang-jwt/jwt v3.2.2+incompatible
github.com/gorilla/securecookie v1.1.1
github.com/gorilla/sessions v1.1.1
github.com/jackc/pgconn v1.13.0
github.com/jackc/pgerrcode v0.0.0-20201024163028-a0d42d470451
github.com/jackc/pgproto3/v2 v2.3.1 // indirect
Expand Down Expand Up @@ -101,7 +99,6 @@ require (
github.com/golang-jwt/jwt/v4 v4.4.3 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/gorilla/context v1.1.1 // indirect
github.com/gorilla/css v1.0.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 // indirect
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect
Expand Down
6 changes: 0 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -249,14 +249,8 @@ github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
github.com/gorilla/context v1.1.1 h1:AWwleXJkX/nhcU9bZSnZoi3h/qGYqQAGhq6zZe/aQW8=
github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
github.com/gorilla/css v1.0.0 h1:BQqNyPTi50JCFMTw/b67hByjMVXZRwGha6wxVGkeihY=
github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl60c=
github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyCS8BvQ=
github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
github.com/gorilla/sessions v1.1.1 h1:YMDmfaK68mUixINzY/XjscuJ47uXFWSSHzFbBQM0PrE=
github.com/gorilla/sessions v1.1.1/go.mod h1:8KCfur6+4Mqcc6S0FEfKuN15Vl5MgXW92AE8ovaJD0w=
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 h1:BZHcxBETFHIdVyhyEfOvn/RdU/QGdLI4y34qQGjGWO0=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0/go.mod h1:hgWBS7lorOAVIJEQMi4ZsPv9hVvWI6+ch50m39Pf2Ks=
Expand Down
7 changes: 0 additions & 7 deletions internal/api/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,6 @@ func (a *API) GetExternalProviderRedirectURL(w http.ResponseWriter, r *http.Requ
}

authURL := p.AuthCodeURL(tokenString, authUrlParams...)
switch externalProvider := p.(type) {
case *provider.TwitterProvider:
if err := storage.StoreInSession(providerType, externalProvider.Marshal(), r, w); err != nil {
return "", internalServerError("Error storing request token in session").WithInternalError(err)
}
}

return authURL, nil
}

Expand Down
17 changes: 3 additions & 14 deletions internal/api/external_oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/sirupsen/logrus"
"github.com/supabase/auth/internal/api/provider"
"github.com/supabase/auth/internal/observability"
"github.com/supabase/auth/internal/storage"
)

// OAuthProviderData contains the userData and token returned by the oauth provider
Expand Down Expand Up @@ -110,24 +109,14 @@ func (a *API) oAuth1Callback(ctx context.Context, r *http.Request, providerType
if err != nil {
return nil, badRequestError("Unsupported provider: %+v", err).WithInternalError(err)
}
value, err := storage.GetFromSession(providerType, r)
if err != nil {
return &OAuthProviderData{}, err
}
oauthToken := getRequestToken(ctx)
oauthVerifier := getOAuthVerifier(ctx)
var accessToken *oauth.AccessToken
var userData *provider.UserProvidedData
if twitterProvider, ok := oAuthProvider.(*provider.TwitterProvider); ok {
requestToken, err := twitterProvider.Unmarshal(value)
if err != nil {
return &OAuthProviderData{}, err
}
if requestToken.Token != oauthToken {
return nil, internalServerError("Request token doesn't match token in callback")
}
twitterProvider.OauthVerifier = oauthVerifier
accessToken, err = twitterProvider.Consumer.AuthorizeToken(requestToken, oauthVerifier)
accessToken, err = twitterProvider.Consumer.AuthorizeToken(&oauth.RequestToken{
Token: oauthToken,
}, oauthVerifier)
if err != nil {
return nil, internalServerError("Unable to retrieve access token").WithInternalError(err)
}
Expand Down
42 changes: 0 additions & 42 deletions internal/storage/session.go

This file was deleted.

0 comments on commit 4f39d2e

Please sign in to comment.