From 56ef055b5db8fcd15394c3665280cb244708ebbf Mon Sep 17 00:00:00 2001
From: Geoffrey Ragot <geoffrey.ragot@gmail.com>
Date: Wed, 3 Aug 2022 09:43:22 +0200
Subject: [PATCH] fix: client secret check

---
 cmd/serve.go           | 43 +++++++++++++++++++-----------------------
 pkg/client.go          | 32 +++++++++++++++++++++++++------
 pkg/storage/client.go  |  2 +-
 pkg/storage/storage.go |  7 +++----
 4 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/cmd/serve.go b/cmd/serve.go
index d7f7a93..dec5fdf 100644
--- a/cmd/serve.go
+++ b/cmd/serve.go
@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/google/uuid"
 	auth "github.com/numary/auth/pkg"
 	"github.com/numary/auth/pkg/api"
 	"github.com/numary/auth/pkg/delegatedauth"
@@ -86,6 +85,23 @@ var serveCmd = &cobra.Command{
 			fx.Invoke(func(lc fx.Lifecycle, db *gorm.DB) {
 				lc.Append(fx.Hook{
 					OnStart: func(ctx context.Context) error {
+						client := &auth.Client{
+							Id: "demo",
+							RedirectURIs: auth.Array[string]{
+								"http://localhost:3000/auth-callback",
+							},
+							ApplicationType: op.ApplicationTypeWeb,
+							AuthMethod:      oidc.AuthMethodNone,
+							ResponseTypes:   []oidc.ResponseType{oidc.ResponseTypeCode},
+							GrantTypes: []oidc.GrantType{
+								oidc.GrantTypeCode,
+								oidc.GrantTypeRefreshToken,
+								oidc.GrantTypeClientCredentials,
+							},
+							AccessTokenType:        op.AccessTokenTypeJWT,
+							PostLogoutRedirectUris: auth.Array[string]{"http://localhost:3000/"},
+						}
+						secret, _ := client.GenerateNewSecretWithClear("default", "1234")
 						return db.
 							WithContext(ctx).
 							Clauses(clause.OnConflict{
@@ -98,31 +114,10 @@ var serveCmd = &cobra.Command{
 									},
 									"post_logout_redirect_uri": `["http://localhost:3000/"]`,
 									"access_token_type":        op.AccessTokenTypeJWT,
-									"secrets":                  `[{"value": "1234"}]`,
+									"secrets":                  fmt.Sprintf(`[{"hash": "%s"}]`, secret.Hash),
 								}),
 							}).
-							Create(&auth.Client{
-								Id: "demo",
-								Secrets: auth.Array[auth.ClientSecret]{
-									{
-										ID:   uuid.NewString(),
-										Hash: "1234",
-									},
-								},
-								RedirectURIs: auth.Array[string]{
-									"http://localhost:3000/auth-callback",
-								},
-								ApplicationType: op.ApplicationTypeWeb,
-								AuthMethod:      oidc.AuthMethodNone,
-								ResponseTypes:   []oidc.ResponseType{oidc.ResponseTypeCode},
-								GrantTypes: []oidc.GrantType{
-									oidc.GrantTypeCode,
-									oidc.GrantTypeRefreshToken,
-									oidc.GrantTypeClientCredentials,
-								},
-								AccessTokenType:        op.AccessTokenTypeJWT,
-								PostLogoutRedirectUris: auth.Array[string]{"http://localhost:3000/"},
-							}).Error
+							Create(client).Error
 					},
 				})
 			}),
diff --git a/pkg/client.go b/pkg/client.go
index 6dca8cc..f0ba67d 100644
--- a/pkg/client.go
+++ b/pkg/client.go
@@ -3,6 +3,7 @@ package auth
 import (
 	"crypto/sha256"
 	"encoding/base64"
+	"fmt"
 	"time"
 
 	"github.com/google/uuid"
@@ -26,17 +27,20 @@ type ClientSecret struct {
 }
 
 func (s ClientSecret) Check(clear string) bool {
+	fmt.Println("check secret", clear, s.Hash, newHash(clear))
 	return s.Hash == newHash(clear)
 }
 
-func newSecret(name string) (ClientSecret, string) {
-	secret := uuid.NewString()
+func newSecret(name, clear string) (ClientSecret, string) {
+	if clear == "" {
+		clear = uuid.NewString()
+	}
 	return ClientSecret{
 		ID:         uuid.NewString(),
-		Hash:       newHash(secret),
-		LastDigits: secret[len(secret)-4:],
+		Hash:       newHash(clear),
+		LastDigits: clear[len(clear)-4:],
 		Name:       name,
-	}, secret
+	}, clear
 }
 
 type Client struct {
@@ -74,12 +78,28 @@ func (c *Client) Update(opts ClientOptions) {
 }
 
 func (c *Client) GenerateNewSecret(name string) (ClientSecret, string) {
-	secret, clear := newSecret(name)
+	secret, clear := newSecret(name, "")
+	c.Secrets = append(c.Secrets, secret)
+
+	return secret, clear
+}
+
+func (c *Client) GenerateNewSecretWithClear(name, clear string) (ClientSecret, string) {
+	secret, clear := newSecret(name, clear)
 	c.Secrets = append(c.Secrets, secret)
 
 	return secret, clear
 }
 
+func (c *Client) HasSecret(clear string) bool {
+	for _, secret := range c.Secrets {
+		if secret.Check(clear) {
+			return true
+		}
+	}
+	return false
+}
+
 func (c *Client) DeleteSecret(id string) bool {
 	for i, secret := range c.Secrets {
 		if secret.ID == id {
diff --git a/pkg/storage/client.go b/pkg/storage/client.go
index 2c538eb..67dc751 100644
--- a/pkg/storage/client.go
+++ b/pkg/storage/client.go
@@ -101,7 +101,7 @@ func (c *clientFacade) RestrictAdditionalAccessTokenScopes() func(scopes []strin
 //IsScopeAllowed enables Client specific custom scopes validation
 func (c *clientFacade) IsScopeAllowed(scope string) bool {
 	scopes := make([]auth.Scope, 0)
-	if err := c.db.First(&scopes, "string_value = ?", scope).Error; err != nil {
+	if err := c.db.First(&scopes, "label = ?", scope).Error; err != nil {
 		switch err {
 		case gorm.ErrRecordNotFound:
 			return false
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 3f20b14..f480d77 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -355,11 +355,10 @@ func (s *storage) AuthorizeClientIDSecret(ctx context.Context, clientID, clientS
 		return err
 	}
 
-	for _, secret := range client.Secrets {
-		if secret.Hash == clientSecret {
-			return nil
-		}
+	if client.HasSecret(clientSecret) {
+		return nil
 	}
+
 	return fmt.Errorf("invalid secret")
 }