Skip to content

Commit

Permalink
Revert "oauth: get signing key from provider (#236)" (#250)
Browse files Browse the repository at this point in the history
This reverts commit 899a32b.

Keeps NewProvider normalization + initializeOauthProviders helper
  • Loading branch information
AlexCuse authored Mar 23, 2024
1 parent 6ac2058 commit a0bb25a
Show file tree
Hide file tree
Showing 19 changed files with 81 additions and 177 deletions.
4 changes: 0 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ Based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## 1.18.1

### Added

* Ability to configure signing key per oauth provider.

### Fixed

* Disallow OAuth linking to an account other than current session's account (#246)
Expand Down
4 changes: 2 additions & 2 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func NewApp(cfg *Config, logger logrus.FieldLogger) (*App, error) {
)
}

oauthProviders := initializeOAuthProviders(cfg)
oauthProviders := initializeOauthProviders(cfg)

return &App{
// Provide access to root DB - useful when extending AccountStore functionality
Expand All @@ -118,7 +118,7 @@ func NewApp(cfg *Config, logger logrus.FieldLogger) (*App, error) {
}, nil
}

func initializeOAuthProviders(cfg *Config) map[string]oauth.Provider {
func initializeOauthProviders(cfg *Config) map[string]oauth.Provider {
oauthProviders := make(map[string]oauth.Provider)
if cfg.GoogleOauthCredentials != nil {
oauthProviders["google"] = *oauth.NewGoogleProvider(cfg.GoogleOauthCredentials)
Expand Down
30 changes: 15 additions & 15 deletions app/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,11 @@ var configurers = []configurer{
return nil
},

// GOOGLE_OAUTH_CREDENTIALS is a credential pair in the format `id:secret:signing_key(optional)`.
// When specified, AuthN will enable routes for Google OAuth signin.
// GOOGLE_OAUTH_CREDENTIALS is a credential pair in the format `id:secret`. When specified,
// AuthN will enable routes for Google OAuth signin.
func(c *Config) error {
if val, ok := os.LookupEnv("GOOGLE_OAUTH_CREDENTIALS"); ok {
credentials, err := oauth.NewCredentials(val, c.OAuthSigningKey)
credentials, err := oauth.NewCredentials(val)
if err == nil {
c.GoogleOauthCredentials = credentials
}
Expand All @@ -599,11 +599,11 @@ var configurers = []configurer{
return nil
},

// GITHUB_OAUTH_CREDENTIALS is a credential pair in the format `id:secret:signing_key(optional)`.
// When specified, AuthN will enable routes for GitHub OAuth signin.
// GITHUB_OAUTH_CREDENTIALS is a credential pair in the format `id:secret`. When specified,
// AuthN will enable routes for GitHub OAuth signin.
func(c *Config) error {
if val, ok := os.LookupEnv("GITHUB_OAUTH_CREDENTIALS"); ok {
credentials, err := oauth.NewCredentials(val, c.OAuthSigningKey)
credentials, err := oauth.NewCredentials(val)
if err == nil {
c.GitHubOauthCredentials = credentials
}
Expand All @@ -612,11 +612,11 @@ var configurers = []configurer{
return nil
},

// FACEBOOK_OAUTH_CREDENTIALS is a credential pair in the format `id:secret:signing_key(optional)`.
// When specified, AuthN will enable routes for Facebook OAuth signin.
// FACEBOOK_OAUTH_CREDENTIALS is a credential pair in the format `id:secret`. When specified,
// AuthN will enable routes for Facebook OAuth signin.
func(c *Config) error {
if val, ok := os.LookupEnv("FACEBOOK_OAUTH_CREDENTIALS"); ok {
credentials, err := oauth.NewCredentials(val, c.OAuthSigningKey)
credentials, err := oauth.NewCredentials(val)
if err == nil {
c.FacebookOauthCredentials = credentials
}
Expand All @@ -625,11 +625,11 @@ var configurers = []configurer{
return nil
},

// DISCORD_OAUTH_CREDENTIALS is a credential pair in the format `id:secret:signing_key(optional)`.
// When specified, AuthN will enable routes for Discord OAuth signin.
// DISCORD_OAUTH_CREDENTIALS is a credential pair in the format `id:secret`. When specified,
// AuthN will enable routes for Discord OAuth signin.
func(c *Config) error {
if val, ok := os.LookupEnv("DISCORD_OAUTH_CREDENTIALS"); ok {
credentials, err := oauth.NewCredentials(val, c.OAuthSigningKey)
credentials, err := oauth.NewCredentials(val)
if err == nil {
c.DiscordOauthCredentials = credentials
}
Expand All @@ -638,11 +638,11 @@ var configurers = []configurer{
return nil
},

// Microsoft_OAUTH_CREDENTIALS is a credential pair in the format `id:secret:signing_key(optional)`.
// When specified, AuthN will enable routes for Microsoft OAuth signin.
// Microsoft_OAUTH_CREDENTIALS is a credential pair in the format `id:secret`. When specified,
// AuthN will enable routes for Discord OAuth signin.
func(c *Config) error {
if val, ok := os.LookupEnv("MICROSOFT_OAUTH_CREDENTIALS"); ok {
credentials, err := oauth.NewCredentials(val, c.OAuthSigningKey)
credentials, err := oauth.NewCredentials(val)
if err == nil {
c.MicrosoftOauthCredentials = credentials
}
Expand Down
6 changes: 3 additions & 3 deletions app/tokens/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ type Claims struct {
}

// Sign converts the claims into a serialized string, signed with HMAC.
func (c *Claims) Sign(signingKey jose.SigningKey) (string, error) {
func (c *Claims) Sign(hmacKey []byte) (string, error) {
signer, err := jose.NewSigner(
signingKey,
jose.SigningKey{Algorithm: jose.HS256, Key: hmacKey},
(&jose.SignerOptions{}).WithType("JWT"),
)
if err != nil {
Expand Down Expand Up @@ -68,7 +68,7 @@ func Parse(tokenStr string, cfg *app.Config, nonce string) (*Claims, error) {
// New creates Claims for a JWT suitable as a state parameter during an OAuth flow.
func New(cfg *app.Config, nonce string, destination string) (*Claims, error) {
return &Claims{
Scope: scope,
Scope: scope,
RequestForgeryProtection: nonce,
Destination: destination,
Claims: jwt.Claims{
Expand Down
9 changes: 4 additions & 5 deletions app/tokens/oauth/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"net/url"
"testing"

"github.com/go-jose/go-jose/v3"
"github.com/keratin/authn-server/app"
"github.com/keratin/authn-server/app/tokens/oauth"
"github.com/stretchr/testify/assert"
Expand All @@ -29,7 +28,7 @@ func TestOAuthToken(t *testing.T) {
assert.True(t, token.Audience.Contains("https://authn.example.com"))
assert.NotEmpty(t, token.IssuedAt)

tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey})
tokenStr, err := token.Sign(cfg.OAuthSigningKey)
require.NoError(t, err)

_, err = oauth.Parse(tokenStr, cfg, nonce)
Expand All @@ -40,7 +39,7 @@ func TestOAuthToken(t *testing.T) {
token, err := oauth.New(cfg, nonce, "https://app.example.com/return")
require.NoError(t, err)

tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey})
tokenStr, err := token.Sign(cfg.OAuthSigningKey)
require.NoError(t, err)

_, err = oauth.Parse(tokenStr, cfg, "wrong")
Expand All @@ -54,7 +53,7 @@ func TestOAuthToken(t *testing.T) {
}
token, err := oauth.New(cfg, nonce, "https://app.example.com/return")
require.NoError(t, err)
tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: oldCfg.OAuthSigningKey})
tokenStr, err := token.Sign(oldCfg.OAuthSigningKey)
require.NoError(t, err)
_, err = oauth.Parse(tokenStr, cfg, nonce)
assert.Error(t, err)
Expand All @@ -67,7 +66,7 @@ func TestOAuthToken(t *testing.T) {
}
token, err := oauth.New(&oldCfg, nonce, "https://app.example.com/return")
require.NoError(t, err)
tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey})
tokenStr, err := token.Sign(cfg.OAuthSigningKey)
require.NoError(t, err)
_, err = oauth.Parse(tokenStr, cfg, nonce)
assert.Error(t, err)
Expand Down
52 changes: 25 additions & 27 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,55 +261,53 @@ or

* `https://www.example.com/authn/oauth/google/return`

By default OAuth providers will use a key derived from `SECRET_KEY_BASE`. To override you can provide a hex-encoded string as the third segment in the colon-delimited environment variable.

### `FACEBOOK_OAUTH_CREDENTIALS`

| | |
|-----------|--------------------------------------|
| Required? | No |
| Value | AppID:AppSecret:SigningKey(optional) |
| Default | nil |
| | |
| --------- | --- |
| Required? | No |
| Value | AppID:AppSecret |
| Default | nil |

Create a Facebook app at https://developers.facebook.com and enable the Facebook Login product. In the Quickstart, enter [AuthN's OAuth Return](api.md#oauth-return) as the Site URL. Then switch over to Settings and find the App ID and Secret. Join those together with a `:` and provide them to AuthN as a single variable.

### `GITHUB_OAUTH_CREDENTIALS`

| | |
|-----------|--------------------------------------|
| Required? | No |
| Value | AppID:AppSecret:SigningKey(optional) |
| Default | nil |
| | |
| --------- | --- |
| Required? | No |
| Value | ClientID:ClientSecret |
| Default | nil |

Sign up for GitHub OAuth 2.0 credentials with the instructions here: https://developer.github.com/apps/building-oauth-apps. Your client's ID and secret must be joined together with a `:` and provided to AuthN as a single variable.

### `GOOGLE_OAUTH_CREDENTIALS`

| | |
|-----------|--------------------------------------|
| Required? | No |
| Value | AppID:AppSecret:SigningKey(optional) |
| Default | nil |
| | |
| --------- | --- |
| Required? | No |
| Value | ClientID:ClientSecret |
| Default | nil |

Sign up for Google OAuth 2.0 credentials with the instructions here: https://developers.google.com/identity/protocols/OpenIDConnect. Your client's ID and secret must be joined together with a `:` and provided to AuthN as a single variable.

### `DISCORD_OAUTH_CREDENTIALS`

| | |
|-----------|--------------------------------------|
| Required? | No |
| Value | AppID:AppSecret:SigningKey(optional) |
| Default | nil |
| | |
| --------- | --- |
| Required? | No |
| Value | ClientID:ClientSecret |
| Default | nil |

Sign up for Discord OAuth 2.0 credentials with the instructions here: https://discordapp.com/developers/docs/topics/oauth2. Your client's ID and secret must be joined together with a `:` and provided to AuthN as a single variable.

### `MICROSOFT_OAUTH_CREDENTIALS`

| | |
|-----------|--------------------------------------|
| Required? | No |
| Value | AppID:AppSecret:SigningKey(optional) |
| Default | nil |
| | |
| --------- | --- |
| Required? | No |
| Value | ClientID:ClientSecret |
| Default | nil |

Sign up for Microsoft OAuth 2.0 credentials with the instructions here: https://docs.microsoft.com/fr-fr/graph/auth/. Your client's ID and secret must be joined together with a `:` and provided to AuthN as a single variable.

Expand Down
36 changes: 10 additions & 26 deletions lib/oauth/credentials.go
Original file line number Diff line number Diff line change
@@ -1,41 +1,25 @@
package oauth

import (
"encoding/hex"
"errors"
"fmt"
"strings"
)

// Credentials is a configuration struct for OAuth Providers
type Credentials struct {
ID string
Secret string
SigningKey []byte
ID string
Secret string
}

// NewCredentials parses a credential string in the format `id:string:signing_key(optional)`
// and returns a Credentials suitable for OAuth Provider configuration. If no signing key is
// provided the default key is used.
func NewCredentials(credentials string, defaultKey []byte) (*Credentials, error) {
if strings.Count(credentials, ":") < 1 {
return nil, errors.New("Credentials must be in the format `id:string:signing_key(optional)`")
// NewCredentials parses a credential string in the format `id:string` and returns a Credentials
// suitable for OAuth Provider configuration.
func NewCredentials(credentials string) (*Credentials, error) {
if strings.Count(credentials, ":") != 1 {
return nil, errors.New("Credentials must be in the format `id:string`")
}
strs := strings.SplitN(credentials, ":", 3)

c := &Credentials{
strs := strings.SplitN(credentials, ":", 2)
return &Credentials{
ID: strs[0],
Secret: strs[1],
}

if len(strs) == 3 {
key, err := hex.DecodeString(strs[2])
if err != nil {
return nil, fmt.Errorf("failed to decode signing key: %w", err)
}
c.SigningKey = key
} else {
c.SigningKey = defaultKey
}
return c, nil
}, nil
}
57 changes: 0 additions & 57 deletions lib/oauth/credentials_test.go

This file was deleted.

3 changes: 1 addition & 2 deletions lib/oauth/discord.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"io"

"github.com/go-jose/go-jose/v3"
"golang.org/x/oauth2"
)

Expand Down Expand Up @@ -37,5 +36,5 @@ func NewDiscordProvider(credentials *Credentials) *Provider {
var user UserInfo
err = json.Unmarshal(body, &user)
return &user, err
}, jose.SigningKey{Key: credentials.SigningKey, Algorithm: jose.HS256})
})
}
3 changes: 1 addition & 2 deletions lib/oauth/facebook.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"io"

"github.com/go-jose/go-jose/v3"
"golang.org/x/oauth2"
"golang.org/x/oauth2/facebook"
)
Expand Down Expand Up @@ -35,5 +34,5 @@ func NewFacebookProvider(credentials *Credentials) *Provider {
var user UserInfo
err = json.Unmarshal(body, &user)
return &user, err
}, jose.SigningKey{Key: credentials.SigningKey, Algorithm: jose.HS256})
})
}
Loading

0 comments on commit a0bb25a

Please sign in to comment.