8000 Use userpassword.Hash · coder/coder@b45413f · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit b45413f

Browse files
code-ashercode-asher
committed
Use userpassword.Hash
1 parent 1c4a2b0 commit b45413f

File tree

5 files changed

+201
-68
lines changed

5 files changed

+201
-68
lines changed

‎enterprise/coderd/identityprovider/authorize.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/coder/coder/v2/coderd/httpapi"
1616
"github.com/coder/coder/v2/coderd/httpmw"
1717
"github.com/coder/coder/v2/codersdk"
18-
"github.com/coder/coder/v2/cryptorand"
1918
)
2019

2120
type authorizeParams struct {
@@ -80,15 +79,13 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
8079
}
8180

8281
// TODO: Ignoring scope for now, but should look into implementing.
83-
// 40 characters matches the length of GitHub's client secrets.
84-
rawCode, err := cryptorand.String(40)
82+
code, err := GenerateSecret()
8583
if err != nil {
8684
httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{
8785
Message: "Failed to generate OAuth2 app authorization code.",
8886
})
8987
return
9088
}
91-
hashedCode := Hash(rawCode, app.ID)
9289
err = db.InTx(func(tx database.Store) error {
9390
// Delete any previous codes.
9491
err = tx.DeleteOAuth2ProviderAppCodesByAppAndUserID(ctx, database.DeleteOAuth2ProviderAppCodesByAppAndUserIDParams{
@@ -105,7 +102,8 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
105102
CreatedAt: dbtime.Now(),
106103
// TODO: Configurable expiration? Ten minutes matches GitHub.
107104
ExpiresAt: dbtime.Now().Add(time.Duration(10) * time.Minute),
108-
HashedSecret: hashedCode[:],
105+
SecretPrefix: []byte(code.Prefix),
106+
HashedSecret: []byte(code.Hashed),
109107
AppID: app.ID,
110108
UserID: apiKey.UserID,
111109
})
@@ -124,7 +122,7 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
124122
}
125123

126124
newQuery := params.redirectURL.Query()
127-
newQuery.Add("code", rawCode)
125+
newQuery.Add("code", code.Formatted)
128126
newQuery.Add("state", params.state)
129127
params.redirectURL.RawQuery = newQuery.Encode()
130128

‎enterprise/coderd/identityprovider/secrets.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package identityprovider
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
7+
"golang.org/x/xerrors"
8+
9+
"github.com/coder/coder/v2/coderd/userpassword"
10+
"github.com/coder/coder/v2/cryptorand"
11+
)
12+
13+
type OAuth2ProviderAppSecret struct {
14+
Formatted string
15+
Prefix string
16+
Hashed string
17+
}
18+
19+
// GenerateSecret generates a secret to be used as a client secret, refresh
20+
// token, or authorization code.
21+
func GenerateSecret() (OAuth2ProviderAppSecret, error) {
22+
// 40 characters matches the length of GitHub's client secrets.
23+
secret, err := cryptorand.String(40)
24+
if err != nil {
25+
return OAuth2ProviderAppSecret{}, err
26+
}
27+
28+
// This ID is prefixed to the secret so it can be used to look up the secret
29+
// when the user provides it, since we cannot just re-hash it to match as we
30+
// will not have the salt.
31+
prefix, err := cryptorand.String(10)
32+
if err != nil {
33+
return OAuth2ProviderAppSecret{}, err
34+
}
35+
36+
hashed, err := userpassword.Hash(secret)
37+
if err != nil {
38+
return OAuth2ProviderAppSecret{}, err
39+
}
40+
41+
return OAuth2ProviderAppSecret{
42+
Formatted: fmt.Sprintf("coder_%s_%s", prefix, secret),
43+
Prefix: prefix,
44+
Hashed: hashed,
45+
}, nil
46+
}
47+
48+
type parsedSecret struct {
49+
prefix string
50+
secret string
51+
}
52+
53+
// parseSecret extracts the ID and original secret from a secret.
54+
func parseSecret(secret string) (parsedSecret, error) {
55+
parts := strings.Split(secret, "_")
56+
if len(parts) != 3 {
57+
return parsedSecret{}, xerrors.Errorf("incorrect number of parts: %d", len(parts))
58+
}
59+
if parts[0] != "coder" {
60+
return parsedSecret{}, xerrors.Errorf("incorrect scheme: %s", parts[0])
61+
}
62+
if len(parts[1]) == 0 {
63+
return parsedSecret{}, xerrors.Errorf("prefix is invalid")
64+
}
65+
if len(parts[2]) == 0 {
66+
return parsedSecret{}, xerrors.Errorf("invalid")
67+
}
68+
return parsedSecret{parts[1], parts[2]}, nil
69+
}

‎enterprise/coderd/identityprovider/tokens.go

Lines changed: 48 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"time"
1111

1212
"github.com/google/uuid"
13-
"golang.org/x/crypto/argon2"
1413
"golang.org/x/oauth2"
1514
"golang.org/x/xerrors"
1615

@@ -21,10 +20,16 @@ import (
2120
"github.com/coder/coder/v2/coderd/httpapi"
2221
"github.com/coder/coder/v2/coderd/httpmw"
2322
"github.com/coder/coder/v2/coderd/rbac"
23+
"github.com/coder/coder/v2/coderd/userpassword"
2424
"github.com/coder/coder/v2/codersdk"
25-
"github.com/coder/coder/v2/cryptorand"
2625
)
2726

27+
// errBadSecret means the user provided a bad secret.
28+
var errBadSecret = xerrors.New("Invalid client secret")
29+
30+
// errBadCode means the user provided a bad code.
31+
var errBadCode = xerrors.New("Invalid code")
32+
2833
type tokenParams struct {
2934
clientID string
3035
clientSecret string
@@ -86,12 +91,12 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
8691
switch params.grantType {
8792
// TODO: Client creds, device code, refresh.
8893
default:
89-
token, err = authorizationCodeGrant(ctx, db, app, defaultLifetime, params.clientSecret, params.code)
94+
token, err = authorizationCodeGrant(ctx, db, app, defaultLifetime, params)
9095
}
9196

92-
if err != nil && errors.Is(err, sql.ErrNoRows) {
97+
if errors.Is(err, errBadCode) || errors.Is(err, errBadSecret) {
9398
httpapi.Write(r.Context(), rw, http.StatusUnauthorized, codersdk.Response{
94-
Message: "Invalid client secret or code",
99+
Message: err.Error(),
95100
})
96101
return
97102
}
@@ -109,47 +114,59 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
109114
}
110115
}
111116

112-
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, clientSecret, code string) (oauth2.Token, error) {
117+
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
113118
// Validate the client secret.
114-
secretHash := Hash(clientSecret, app.ID)
115-
secret, err := db.GetOAuth2ProviderAppSecretByAppIDAndSecret(
116-
//nolint:gocritic // Users cannot read secrets so we must use the system.
117-
dbauthz.AsSystemRestricted(ctx),
118-
database.GetOAuth2ProviderAppSecretByAppIDAndSecretParams{
119-
AppID: app.ID,
120-
HashedSecret: secretHash[:],
121-
})
119+
secret, err := parseSecret(params.clientSecret)
120+
if err != nil {
121+
return oauth2.Token{}, errBadSecret
122+
}
123+
//nolint:gocritic // Users cannot read secrets so we must use the system.
124+
dbSecret, err := db.GetOAuth2ProviderAppSecretByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(secret.prefix))
125+
if errors.Is(err, sql.ErrNoRows) {
126+
return oauth2.Token{}, errBadSecret
127+
}
122128
if err != nil {
123129
return oauth2.Token{}, err
124130
}
131+
equal, err := userpassword.Compare(string(dbSecret.HashedSecret), secret.secret)
132+
if err != nil {
133+
return oauth2.Token{}, xerrors.Errorf("unable to compare secret: %w", err)
134+
}
135+
if !equal {
136+
return oauth2.Token{}, errBadSecret
137+
}
125138

126139
// Validate the authorization code.
127-
codeHash := Hash(code, app.ID)
140+
code, err := parseSecret(params.code)
128141
if err != nil {
129-
return oauth2.Token{}, err
142+
return oauth2.Token{}, errBadCode
143+
}
144+
//nolint:gocritic // There is no user yet so we must use the system.
145+
dbCode, err := db.GetOAuth2ProviderAppCodeByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(code.prefix))
146+
if errors.Is(err, sql.ErrNoRows) {
147+
return oauth2.Token{}, errBadCode
130148
}
131-
dbCode, err := db.GetOAuth2ProviderAppCodeByAppIDAndSecret(
132-
//nolint:gocritic // There is no user yet so we must use the system.
133-
dbauthz.AsSystemRestricted(ctx),
134-
database.GetOAuth2ProviderAppCodeByAppIDAndSecretParams{
135-
AppID: app.ID,
136-
HashedSecret: codeHash[:],
137-
})
138149
if err != nil {
139150
return oauth2.Token{}, err
140151
}
152+
equal, err = userpassword.Compare(string(dbCode.HashedSecret), code.secret)
153+
if err != nil {
154+
return oauth2.Token{}, xerrors.Errorf("unable to compare code: %w", err)
155+
}
156+
if !equal {
157+
return oauth2.Token{}, errBadCode
158+
}
141159

142-
// Ensure the code has not expired. Make it look like no code.
160+
// Ensure the code has not expired.
143161
if dbCode.ExpiresAt.Before(dbtime.Now()) {
144-
return oauth2.Token{}, sql.ErrNoRows
162+
return oauth2.Token{}, errBadCode
145163
}
146164

147165
// Generate a refresh token.
148166
// The refresh token is not currently used or exposed though as API keys can
149167
// already be refreshed by just using them.
150168
// TODO: However, should we implement the refresh grant anyway?
151-
// 40 characters matches the length of GitHub's client secrets.
152-
rawRefreshToken, err := cryptorand.String(40)
169+
refreshToken, err := GenerateSecret()
153170
if err != nil {
154171
return oauth2.Token{}, err
155172
}
@@ -208,13 +225,13 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
208225
return xerrors.Errorf("insert oauth2 access token: %w", err)
209226
}
210227

211-
hashed := Hash(rawRefreshToken, app.ID)
212228
_, err = tx.InsertOAuth2ProviderAppToken(ctx, database.InsertOAuth2ProviderAppTokenParams{
213229
ID: uuid.New(),
214230
CreatedAt: dbtime.Now(),
215231
ExpiresAt: key.ExpiresAt,
216-
RefreshHash: hashed[:],
217-
AppSecretID: secret.ID,
232+
HashPrefix: []byte(refreshToken.Prefix),
233+
RefreshHash: []byte(refreshToken.Hashed),
234+
AppSecretID: dbSecret.ID, F438
218235
APIKeyID: newKey.ID,
219236
})
220237
if err != nil {
@@ -230,16 +247,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
230247
AccessToken: sessionToken,
231248
TokenType: "Bearer",
232249
// TODO: Exclude until refresh grant is implemented.
233-
// RefreshToken: rawRefreshToken,
250+
// RefreshToken: refreshToken.formatted,
234251
// Expiry: key.ExpiresAt,
235252
}, nil
236253
}
237-
238-
/**
239-
* Hash uses argon2 to hash the secret using the ID as the salt.
240-
*/
241-
func Hash(secret string, id uuid.UUID) []byte {
242-
b := []byte(secret)
243-
// TODO: Expose iterations, memory, and threads as configuration values?
244-
return argon2.IDKey(b, []byte(id.String()), 1, 64*1024, 2, uint32(len(b)))
245-
}

‎enterprise/coderd/oauth2.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/coder/coder/v2/coderd/httpapi"
1414
"github.com/coder/coder/v2/coderd/httpmw"
1515
"github.com/coder/coder/v2/codersdk"
16-
"github.com/coder/coder/v2/cryptorand"
1716
"github.com/coder/coder/v2/enterprise/coderd/identityprovider"
1817
)
1918

@@ -233,24 +232,23 @@ func (api *API) oAuth2ProviderAppSecrets(rw http.ResponseWriter, r *http.Request
233232
func (api *API) postOAuth2ProviderAppSecret(rw http.ResponseWriter, r *http.Request) {
234233
ctx := r.Context()
235234
app := httpmw.OAuth2ProviderApp(r)
236-
// 40 characters matches the length of GitHub's client secrets.
237-
rawSecret, err := cryptorand.String(40)
235+
secret, err := identityprovider.GenerateSecret()
238236
if err != nil {
239-
httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{
237+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
240238
Message: "Failed to generate OAuth2 client secret.",
239+
Detail: err.Error(),
241240
})
242241
return
243242
}
244-
hashed := identityprovider.Hash(rawSecret, app.ID)
245-
secret, err := api.Database.InsertOAuth2ProviderAppSecret(ctx, database.InsertOAuth2ProviderAppSecretParams{
243+
dbSecret, err := api.Database.InsertOAuth2ProviderAppSecret(ctx, database.InsertOAuth2ProviderAppSecretParams{
246244
ID: uuid.New(),
247245
CreatedAt: dbtime.Now(),
248-
SecretPrefix: []byte(""), // TODO: Currently unused.
249-
HashedSecret: hashed[:],
246+
SecretPrefix: []byte(secret.Prefix),
247+
HashedSecret: []byte(secret.Hashed),
250248
// DisplaySecret is the last six characters of the original unhashed secret.
251249
// This is done so they can be differentiated and it matches how GitHub
252250
// displays their client secrets.
253-
DisplaySecret: rawSecret[len(rawSecret)-6:],
251+
DisplaySecret: secret.Formatted[len(secret.Formatted)-6:],
254252
AppID: app.ID,
255253
})
256254
if err != nil {
@@ -261,8 +259,8 @@ func (api *API) postOAuth2ProviderAppSecret(rw http.ResponseWriter, r *http.Requ
261259
return
262260
}
263261
httpapi.Write(ctx, rw, http.StatusOK, codersdk.OAuth2ProviderAppSecretFull{
264-
ID: secret.ID,
265-
ClientSecretFull: rawSecret,
262+
ID: dbSecret.ID,
263+
ClientSecretFull: secret.Formatted,
266264
})
267265
}
268266

0 commit comments

Comments
 (0)
0