Skip to content

Commit d5878e5

Browse files
louischan-ourskylouischan-oursky
committed
Implement client secret rotation
ref DEV-2905
2 parents 164041e + 2e58633 commit d5878e5

22 files changed

Lines changed: 862 additions & 256 deletions

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ export-schemas:
175175
go run ./scripts/exportschemas -s secrets-config -o tmp/secrets-config.schema.json
176176
npm run --silent --prefix ./scripts/npm export-graphql-schema admin > portal/src/graphql/adminapi/schema.graphql
177177
npm run --silent --prefix ./scripts/npm export-graphql-schema portal > portal/src/graphql/portal/schema.graphql
178+
cd portal && npm run gentype
178179

179180
.PHONY: export-v2-translations
180181
export-v2-translations:

‎pkg/lib/config/secret_update_instruction.go‎

Lines changed: 97 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,11 @@ type OAuthClientSecretsUpdateInstructionGenerateData struct {
276276
ClientID string `json:"clientID,omitempty"`
277277
}
278278

279+
type OAuthClientSecretsUpdateInstructionDeleteData struct {
280+
ClientID string `json:"clientID,omitempty"`
281+
KeyID string `json:"keyID,omitempty"`
282+
}
283+
279284
type OAuthClientSecretsUpdateInstructionCleanupData struct {
280285
KeepClientIDs []string `json:"keepClientIDs,omitempty"`
281286
}
@@ -284,13 +289,16 @@ type OAuthClientSecretsUpdateInstruction struct {
284289
Action SecretUpdateInstructionAction `json:"action,omitempty"`
285290

286291
GenerateData *OAuthClientSecretsUpdateInstructionGenerateData `json:"generateData,omitempty"`
292+
DeleteData *OAuthClientSecretsUpdateInstructionDeleteData `json:"deleteData,omitempty"`
287293
CleanupData *OAuthClientSecretsUpdateInstructionCleanupData `json:"cleanupData,omitempty"`
288294
}
289295

290296
func (i *OAuthClientSecretsUpdateInstruction) ApplyTo(ctx *SecretConfigUpdateInstructionContext, currentConfig *SecretConfig) (*SecretConfig, error) {
291297
switch i.Action {
292298
case SecretUpdateInstructionActionGenerate:
293299
return i.generate(ctx, currentConfig)
300+
case SecretUpdateInstructionActionDelete:
301+
return i.delete(currentConfig)
294302
case SecretUpdateInstructionActionCleanup:
295303
return i.cleanup(currentConfig)
296304
default:
@@ -320,31 +328,37 @@ func (i *OAuthClientSecretsUpdateInstruction) generate(ctx *SecretConfigUpdateIn
320328

321329
clientID := i.GenerateData.ClientID
322330
jwkKey := ctx.GenerateClientSecretOctetKeyFunc(ctx.Clock.NowUTC(), corerand.SecureRand)
323-
keySet := jwk.NewSet()
324-
_ = keySet.AddKey(jwkKey)
325-
newCredentialsItem := OAuthClientCredentialsItem{
326-
ClientID: clientID,
327-
OAuthClientCredentialsKeySet: OAuthClientCredentialsKeySet{Set: keySet},
328-
}
329331

330332
newOAuthClientCredentials := &OAuthClientCredentials{}
331333
idx, item, found := out.Lookup(OAuthClientCredentialsKey)
334+
// If the secret exist, reuse existing item
332335
if found {
333336
oauth, err := i.decodeOAuthClientCredentials(item.RawData)
334337
if err != nil {
335338
return nil, err
336339
}
337-
_, ok := oauth.Lookup(clientID)
338-
if ok {
339-
return nil, fmt.Errorf("config: client secret already exist")
340+
newOAuthClientCredentials = oauth
341+
}
342+
343+
// Find the existing client item by index
344+
var clientItem *OAuthClientCredentialsItem
345+
if existingClientItem, ok := newOAuthClientCredentials.Lookup(clientID); ok {
346+
clientItem = existingClientItem
347+
} else {
348+
newClientItem := OAuthClientCredentialsItem{
349+
ClientID: clientID,
350+
OAuthClientCredentialsKeySet: OAuthClientCredentialsKeySet{Set: jwk.NewSet()},
340351
}
341-
// copy oauth client secret items from the current config to new config
342-
newOAuthClientCredentials.Items = make([]OAuthClientCredentialsItem, len(oauth.Items))
343-
copy(newOAuthClientCredentials.Items, oauth.Items)
352+
newOAuthClientCredentials.Items = append(newOAuthClientCredentials.Items, newClientItem)
353+
clientItem = &newClientItem
354+
}
355+
356+
// Append the new key
357+
if clientItem.OAuthClientCredentialsKeySet.Len() >= 2 {
358+
return nil, fmt.Errorf("config: must have at most two OAuth client secrets for client %s", clientID)
344359
}
360+
_ = clientItem.OAuthClientCredentialsKeySet.AddKey(jwkKey)
345361

346-
// Add new credentials item to the OAuthClientCredentials
347-
newOAuthClientCredentials.Items = append(newOAuthClientCredentials.Items, newCredentialsItem)
348362
var jsonData []byte
349363
jsonData, err := json.Marshal(newOAuthClientCredentials)
350364
if err != nil {
@@ -364,6 +378,75 @@ func (i *OAuthClientSecretsUpdateInstruction) generate(ctx *SecretConfigUpdateIn
364378
return out, nil
365379
}
366380

381+
func (i *OAuthClientSecretsUpdateInstruction) delete(currentConfig *SecretConfig) (*SecretConfig, error) {
382+
out := &SecretConfig{}
383+
out.Secrets = make([]SecretItem, len(currentConfig.Secrets))
384+
copy(out.Secrets, currentConfig.Secrets)
385+
386+
if i.DeleteData == nil || i.DeleteData.ClientID == "" || i.DeleteData.KeyID == "" {
387+
return nil, fmt.Errorf("config: missing clientID or keyID for OAuthClientSecretsUpdateInstruction")
388+
}
389+
390+
clientID := i.DeleteData.ClientID
391+
keyID := i.DeleteData.KeyID
392+
393+
idx, item, found := out.Lookup(OAuthClientCredentialsKey)
394+
if !found {
395+
return out, nil
396+
}
397+
oauth, err := i.decodeOAuthClientCredentials(item.RawData)
398+
if err != nil {
399+
return nil, err
400+
}
401+
402+
ctx := contextForTheUnusedContextArgumentInJWXV2API
403+
var newOAuthClientCredentialsItems []OAuthClientCredentialsItem
404+
for _, existingItem := range oauth.Items {
405+
if existingItem.ClientID == clientID {
406+
var foundKey jwk.Key
407+
for it := existingItem.OAuthClientCredentialsKeySet.Set.Keys(ctx); it.Next(ctx); {
408+
key := it.Pair().Value.(jwk.Key)
409+
if key.KeyID() == keyID {
410+
foundKey = key
411+
break
412+
}
413+
}
414+
415+
if foundKey != nil {
416+
err := existingItem.OAuthClientCredentialsKeySet.Set.RemoveKey(foundKey)
417+
if err != nil {
418+
return nil, err
419+
}
420+
}
421+
422+
// Check length
423+
if existingItem.OAuthClientCredentialsKeySet.Len() == 0 {
424+
return nil, fmt.Errorf("config: cannot delete the last secret for client %s", clientID)
425+
}
426+
427+
newOAuthClientCredentialsItems = append(newOAuthClientCredentialsItems, existingItem)
428+
} else {
429+
newOAuthClientCredentialsItems = append(newOAuthClientCredentialsItems, existingItem)
430+
}
431+
}
432+
newOAuthClientCredentials := &OAuthClientCredentials{
433+
Items: newOAuthClientCredentialsItems,
434+
}
435+
436+
var jsonData []byte
437+
jsonData, err = json.Marshal(newOAuthClientCredentials)
438+
if err != nil {
439+
return nil, err
440+
}
441+
newSecretItem := SecretItem{
442+
Key: OAuthClientCredentialsKey,
443+
RawData: json.RawMessage(jsonData),
444+
}
445+
out.Secrets[idx] = newSecretItem
446+
447+
return out, nil
448+
}
449+
367450
func (i *OAuthClientSecretsUpdateInstruction) cleanup(currentConfig *SecretConfig) (*SecretConfig, error) {
368451
out := &SecretConfig{}
369452
out.Secrets = make([]SecretItem, len(currentConfig.Secrets))

‎pkg/lib/config/testdata/secret_update_instruction.yaml‎

Lines changed: 130 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,8 @@ updateInstructionJSON: |-
277277
}
278278
}
279279
---
280-
name: generate-oauth-client-secrets-already-exists
281-
error: |-
282-
config: client secret already exist
280+
name: generate-oauth-client-secrets-second-secret
281+
error: null
283282
currentSecretConfigYAML: |-
284283
secrets:
285284
- key: db
@@ -291,10 +290,61 @@ currentSecretConfigYAML: |-
291290
items:
292291
- client_id: "client-id"
293292
keys:
293+
- created_at: 1136214245
294+
k: c2VjcmV0MQ
295+
kid: kid1
296+
kty: oct
297+
newSecretConfigYAML: |-
298+
secrets:
299+
- key: db
300+
data:
301+
database_url: "postgres://postgres@127.0.0.1:5432/postgres"
302+
database_schema: app
303+
- key: oauth.client_secrets
304+
data:
305+
items:
306+
- client_id: "client-id"
307+
keys:
308+
- created_at: 1136214245
309+
k: c2VjcmV0MQ
310+
kid: kid1
311+
kty: oct
294312
- created_at: 1136214245
295313
k: c2VjcmV0MQ
296314
kid: kid
297315
kty: oct
316+
updateInstructionJSON: |-
317+
{
318+
"oauthClientSecrets": {
319+
"action": "generate",
320+
"generateData": {
321+
"clientID": "client-id"
322+
}
323+
}
324+
}
325+
---
326+
name: generate-oauth-client-secrets-too-many-keys
327+
error: |-
328+
config: must have at most two OAuth client secrets for client client-id
329+
currentSecretConfigYAML: |-
330+
secrets:
331+
- key: db
332+
data:
333+
database_url: "postgres://postgres@127.0.0.1:5432/postgres"
334+
database_schema: app
335+
- key: oauth.client_secrets
336+
data:
337+
items:
338+
- client_id: "client-id"
339+
keys:
340+
- created_at: 1136214245
341+
k: c2VjcmV0MQ
342+
kid: kid1
343+
kty: oct
344+
- created_at: 1136214245
345+
k: c2VjcmV0MQ
346+
kid: kid2
347+
kty: oct
298348
newSecretConfigYAML: ""
299349
updateInstructionJSON: |-
300350
{
@@ -434,6 +484,83 @@ updateInstructionJSON: |-
434484
}
435485
}
436486
---
487+
name: delete-oauth-client-secret
488+
error: null
489+
currentSecretConfigYAML: |-
490+
secrets:
491+
- key: db
492+
data:
493+
database_url: "postgres://postgres@127.0.0.1:5432/postgres"
494+
database_schema: app
495+
- key: oauth.client_secrets
496+
data:
497+
items:
498+
- client_id: "client-id"
499+
keys:
500+
- created_at: 1136214245
501+
k: c2VjcmV0MQ
502+
kid: kid1
503+
kty: oct
504+
- created_at: 1136214245
505+
k: c2VjcmV0MQ
506+
kid: kid2
507+
kty: oct
508+
newSecretConfigYAML: |-
509+
secrets:
510+
- key: db
511+
data:
512+
database_url: "postgres://postgres@127.0.0.1:5432/postgres"
513+
database_schema: app
514+
- key: oauth.client_secrets
515+
data:
516+
items:
517+
- client_id: "client-id"
518+
keys:
519+
- created_at: 1136214245
520+
k: c2VjcmV0MQ
521+
kid: kid2
522+
kty: oct
523+
updateInstructionJSON: |-
524+
{
525+
"oauthClientSecrets": {
526+
"action": "delete",
527+
"deleteData": {
528+
"clientID": "client-id",
529+
"keyID": "kid1"
530+
}
531+
}
532+
}
533+
---
534+
name: delete-oauth-client-secret-deleting-only-key
535+
error: |-
536+
config: cannot delete the last secret for client client-id
537+
currentSecretConfigYAML: |-
538+
secrets:
539+
- key: db
540+
data:
541+
database_url: "postgres://postgres@127.0.0.1:5432/postgres"
542+
database_schema: app
543+
- key: oauth.client_secrets
544+
data:
545+
items:
546+
- client_id: "client-id"
547+
keys:
548+
- created_at: 1136214245
549+
k: c2VjcmV0MQ
550+
kid: kid1
551+
kty: oct
552+
newSecretConfigYAML: ""
553+
updateInstructionJSON: |-
554+
{
555+
"oauthClientSecrets": {
556+
"action": "delete",
557+
"deleteData": {
558+
"clientID": "client-id",
559+
"keyID": "kid1"
560+
}
561+
}
562+
}
563+
---
437564
name: generate-admin-api-auth-key
438565
error: null
439566
currentSecretConfigYAML: |-

‎pkg/portal/graphql/app_mutation.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,18 @@ var oauthClientSecretsGenerateDataInput = graphql.NewInputObject(graphql.InputOb
8686
},
8787
})
8888

89+
var oauthClientSecretsDeleteDataInput = graphql.NewInputObject(graphql.InputObjectConfig{
90+
Name: "OAuthClientSecretsDeleteDataInput",
91+
Fields: graphql.InputObjectConfigFieldMap{
92+
"clientID": &graphql.InputObjectFieldConfig{
93+
Type: graphql.NewNonNull(graphql.String),
94+
},
95+
"keyID": &graphql.InputObjectFieldConfig{
96+
Type: graphql.NewNonNull(graphql.String),
97+
},
98+
},
99+
})
100+
89101
var oauthClientSecretsCleanupDataInput = graphql.NewInputObject(graphql.InputObjectConfig{
90102
Name: "OAuthClientSecretsCleanupDataInput",
91103
Fields: graphql.InputObjectConfigFieldMap{
@@ -222,6 +234,9 @@ var oauthClientSecretsUpdateInstructionsInput = graphql.NewInputObject(graphql.I
222234
"generateData": &graphql.InputObjectFieldConfig{
223235
Type: oauthClientSecretsGenerateDataInput,
224236
},
237+
"deleteData": &graphql.InputObjectFieldConfig{
238+
Type: oauthClientSecretsDeleteDataInput,
239+
},
225240
"cleanupData": &graphql.InputObjectFieldConfig{
226241
Type: oauthClientSecretsCleanupDataInput,
227242
},

‎portal/src/TextFieldWithCopyButton.tsx‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ import styles from "./TextFieldWithCopyButton.module.css";
88

99
export interface TextFieldWithCopyButtonProps extends TextFieldProps {
1010
additionalIconButtons?: IButtonProps[];
11+
hideCopyButton?: boolean;
1112
}
1213

1314
const TextFieldWithCopyButton: React.VFC<TextFieldWithCopyButtonProps> =
1415
function TextFieldWithCopyButton(props: TextFieldWithCopyButtonProps) {
15-
const { disabled, additionalIconButtons, ...rest } = props;
16+
const { disabled, additionalIconButtons, hideCopyButton, ...rest } = props;
1617
const { themes } = useSystemConfig();
1718
const { copyButtonProps, Feedback } = useCopyFeedback({
1819
textToCopy: props.value ?? "",
@@ -21,15 +22,19 @@ const TextFieldWithCopyButton: React.VFC<TextFieldWithCopyButtonProps> =
2122
return (
2223
<div className={styles.container}>
2324
<TextField className={styles.textField} disabled={disabled} {...rest} />
24-
<IconButton
25-
{...copyButtonProps}
26-
className={cn(
27-
styles.actionButton,
28-
disabled ? styles["actionButton--hide"] : null
29-
)}
30-
theme={themes.actionButton}
31-
/>
32-
<Feedback />
25+
{!hideCopyButton ? (
26+
<>
27+
<IconButton
28+
{...copyButtonProps}
29+
className={cn(
30+
styles.actionButton,
31+
disabled ? styles["actionButton--hide"] : null
32+
)}
33+
theme={themes.actionButton}
34+
/>
35+
<Feedback />
36+
</>
37+
) : null}
3338
{additionalIconButtons?.map((props, idx) => {
3439
const { className, ...restProps } = props;
3540
return (

0 commit comments

Comments
 (0)