diff --git a/.gitignore b/.gitignore index 45197c7f..52a17382 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,4 @@ test/integration/testfile.txt # Direnv files, because these should be opt-in by the developer. .direnv .envrc +opkssh diff --git a/commands/login.go b/commands/login.go index a1f9345b..c6d33357 100644 --- a/commands/login.go +++ b/commands/login.go @@ -813,9 +813,13 @@ func IdentityString(pkt pktoken.PKToken) (string, error) { } claims := idt.GetClaims() if claims.Email == "" { - return "Sub, issuer, audience: \n" + claims.Subject + " " + claims.Issuer + " " + claims.Audience, nil + return fmt.Sprintf(`WARNING: Email claim is missing from ID token. Policies based on email will not work. +Check if your client config (~/.opk/config.yml) has the correct scopes configured for this OpenID Provider. +Sub, issuer, audience: +%s %s %s`, claims.Subject, claims.Issuer, claims.Audience), nil } else { - return "Email, sub, issuer, audience: \n" + claims.Email + " " + claims.Subject + " " + claims.Issuer + " " + claims.Audience, nil + return fmt.Sprintf(`Email, sub, issuer, audience: +%s %s %s %s`, claims.Email, claims.Subject, claims.Issuer, claims.Audience), nil } } diff --git a/commands/login_test.go b/commands/login_test.go index 953ac9c2..420c3d6e 100644 --- a/commands/login_test.go +++ b/commands/login_test.go @@ -57,7 +57,7 @@ const providerStr3 = providerAlias3 + "," + providerArg3 const allProvidersStr = providerStr1 + ";" + providerStr2 + ";" + providerStr3 -func Mocks(t *testing.T, keyType KeyType) (*pktoken.PKToken, crypto.Signer, providers.OpenIdProvider) { +func Mocks(t *testing.T, keyType KeyType, extraClaims ...map[string]any) (*pktoken.PKToken, crypto.Signer, providers.OpenIdProvider) { var err error var alg jwa.SignatureAlgorithm var signer crypto.Signer @@ -76,9 +76,14 @@ func Mocks(t *testing.T, keyType KeyType) (*pktoken.PKToken, crypto.Signer, prov op, _, idtTemplate, err := providers.NewMockProvider(providerOpts) require.NoError(t, err) - mockEmail := "arthur.aardvark@example.com" - idtTemplate.ExtraClaims = map[string]any{ - "email": mockEmail, + // Default: include email claim + if len(extraClaims) > 0 { + idtTemplate.ExtraClaims = extraClaims[0] + } else { + mockEmail := "arthur.aardvark@example.com" + idtTemplate.ExtraClaims = map[string]any{ + "email": mockEmail, + } } client, err := client.New(op, client.WithSigner(signer, alg)) @@ -415,11 +420,27 @@ func TestCreateSSHCert(t *testing.T) { } func TestIdentityString(t *testing.T) { - pkt, _, _ := Mocks(t, ECDSA) - idString, err := IdentityString(*pkt) - require.NoError(t, err) - expIdString := "Email, sub, issuer, audience: \narthur.aardvark@example.com me https://accounts.example.com test_client_id" - require.Equal(t, expIdString, idString) + t.Run("with email claim", func(t *testing.T) { + pkt, _, _ := Mocks(t, ECDSA) + idString, err := IdentityString(*pkt) + require.NoError(t, err) + expIdString := "Email, sub, issuer, audience: \narthur.aardvark@example.com me https://accounts.example.com test_client_id" + require.Equal(t, expIdString, idString) + }) + + t.Run("without email claim", func(t *testing.T) { + // Create a mock without email claim by passing empty ExtraClaims + pkt, _, _ := Mocks(t, ECDSA, map[string]any{}) + + idString, err := IdentityString(*pkt) + require.NoError(t, err) + require.Contains(t, idString, "WARNING: Email claim is missing from ID token") + require.Contains(t, idString, "Policies based on email will not work") + require.Contains(t, idString, "Sub, issuer, audience:") + require.Contains(t, idString, "me") // subject + require.Contains(t, idString, "https://accounts.example.com") // issuer + require.Contains(t, idString, "test_client_id") // audience + }) } func TestPrettyPrintIdToken(t *testing.T) {