Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Display IDP name when prompting for username and password #1691

Merged
merged 3 commits into from
Oct 10, 2023
Merged

Display IDP name when prompting for username and password #1691

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ee1e35
Rename username and password prompt variables
cfryanr Oct 9, 2023
3a21c9a
backfill unit tests for expected stderr output in login_test.go
cfryanr Oct 9, 2023
0a47aa5
add a login banner to CLI-based login prompts which shows the IDP name
cfryanr Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add a login banner to CLI-based login prompts which shows the IDP name 
Skip showing the banner when the CLI does not know the IDP name
from the CLI args (which are typically encoded in the kubeconfig).

Co-authored-by: Joshua Casey <joshuatcasey@gmail.com>
  • Loading branch information
@cfryanr @joshuatcasey
cfryanr and joshuatcasey committed Oct 10, 2023
commit 0a47aa59fc680b4ef73705951d0f006a0960948f
4 changes: 4 additions & 0 deletions pkg/oidcclient/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@
func (h *handlerState) getUsernameAndPassword() (string, string, error) {
var err error

if h.upstreamIdentityProviderName != "" {
_, _ = fmt.Fprintf(h.out, "\nLog in to %s\n\n", h.upstreamIdentityProviderName)
}

username := h.getEnv(defaultUsernameEnvVarName)
if username == "" {
username, err = h.promptForValue(h.ctx, usernamePrompt, h.out)
Expand Down Expand Up @@ -644,11 +648,11 @@
return wg.Wait
}

func promptForValue(ctx context.Context, promptLabel string, out io.Writer) (string, error) {

Check warning on line 651 in pkg/oidcclient/login.go

View check run for this annotation

Codecov / codecov/patch

pkg/oidcclient/login.go#L651 

Added line #L651 was not covered by tests
if !term.IsTerminal(stdin()) {
return "", errors.New("stdin is not connected to a terminal")
}
_, err := fmt.Fprint(out, promptLabel)

Check warning on line 655 in pkg/oidcclient/login.go

View check run for this annotation

Codecov / codecov/patch

pkg/oidcclient/login.go#L655 

Added line #L655 was not covered by tests
if err != nil {
return "", fmt.Errorf("could not print prompt to stderr: %w", err)
}
Expand Down Expand Up @@ -676,11 +680,11 @@
}
}

func promptForSecret(promptLabel string, out io.Writer) (string, error) {

Check warning on line 683 in pkg/oidcclient/login.go

View check run for this annotation

Codecov / codecov/patch

pkg/oidcclient/login.go#L683 

Added line #L683 was not covered by tests
if !term.IsTerminal(stdin()) {
return "", errors.New("stdin is not connected to a terminal")
}
_, err := fmt.Fprint(out, promptLabel)

Check warning on line 687 in pkg/oidcclient/login.go

View check run for this annotation

Codecov / codecov/patch

pkg/oidcclient/login.go#L687 

Added line #L687 was not covered by tests
if err != nil {
return "", fmt.Errorf("could not print prompt to stderr: %w", err)
}
Expand All @@ -691,7 +695,7 @@
// term.ReadPassword swallows the newline that was typed by the user, so to
// avoid the next line of output from happening on same line as the password
// prompt, we need to print a newline.
_, err = fmt.Fprint(out, "\n")

Check warning on line 698 in pkg/oidcclient/login.go

View check run for this annotation

Codecov / codecov/patch

pkg/oidcclient/login.go#L698 

Added line #L698 was not covered by tests
if err != nil {
return "", fmt.Errorf("could not print newline to stderr: %w", err)
}
Expand Down
71 changes: 41 additions & 30 deletions pkg/oidcclient/login_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,9 +1080,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
return nil
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: "error prompting for username: some prompt error",
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: "error prompting for username: some prompt error",
},
{
name: "ldap login when prompting for password returns an error",
Expand All @@ -1094,9 +1095,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
return nil
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: "error prompting for password: some prompt error",
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: "error prompting for password: some prompt error",
},
{
name: "ldap login when there is a problem with parsing the authorize URL",
Expand Down Expand Up @@ -1149,8 +1151,9 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
return defaultLDAPTestOpts(t, h, nil, errors.New("some error fetching authorize endpoint"))
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: `authorization response error: Get "https://` + successServer.Listener.Addr().String() +
`/authorize?access_type=offline&client_id=test-client-id&code_challenge=` + testCodeChallenge +
`&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=some-upstream-name&` +
Expand All @@ -1165,9 +1168,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
return defaultLDAPTestOpts(t, h, &http.Response{StatusCode: http.StatusBadGateway, Status: "502 Bad Gateway"}, nil)
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: `error getting authorization: expected to be redirected, but response status was 502 Bad Gateway`,
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: `error getting authorization: expected to be redirected, but response status was 502 Bad Gateway`,
},
{
name: "ldap login when the OIDC provider authorization endpoint redirect has an error and error description",
Expand All @@ -1182,9 +1186,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
}, nil)
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: `login failed with code "access_denied": optional-error-description`,
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: `login failed with code "access_denied": optional-error-description`,
},
{
name: "ldap login when the OIDC provider authorization endpoint redirects us to a different server",
Expand All @@ -1199,9 +1204,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
}, nil)
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: `error getting authorization: redirected to the wrong location: http://other-server.example.com/callback?code=foo&state=test-state`,
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: `error getting authorization: redirected to the wrong location: http://other-server.example.com/callback?code=foo&state=test-state`,
},
{
name: "ldap login when the OIDC provider authorization endpoint redirect has an error but no error description",
Expand All @@ -1216,9 +1222,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
}, nil)
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: `login failed with code "access_denied"`,
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: `login failed with code "access_denied"`,
},
{
name: "ldap login when the OIDC provider authorization endpoint redirect has the wrong state value",
Expand All @@ -1231,9 +1238,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
}, nil)
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: `missing or invalid state parameter in authorization response: http://127.0.0.1:0/callback?code=foo&state=wrong-state`,
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: `missing or invalid state parameter in authorization response: http://127.0.0.1:0/callback?code=foo&state=wrong-state`,
},
{
name: "ldap login when there is an error exchanging the authcode or validating the tokens",
Expand All @@ -1258,9 +1266,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
return nil
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantErr: "could not complete authorization code exchange: some authcode exchange or token validation error",
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantErr: "could not complete authorization code exchange: some authcode exchange or token validation error",
},
{
name: "successful ldap login with prompts for username and password",
Expand Down Expand Up @@ -1356,9 +1365,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
return nil
}
},
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantToken: &testToken,
issuer: successServer.URL,
wantLogs: []string{"\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\""},
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantToken: &testToken,
},
{
name: "successful ldap login with env vars for username and password",
Expand Down Expand Up @@ -1572,7 +1582,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
"\"level\"=4 \"msg\"=\"Pinniped: Read username from environment variable\" \"name\"=\"PINNIPED_USERNAME\"",
"\"level\"=4 \"msg\"=\"Pinniped: Read password from environment variable\" \"name\"=\"PINNIPED_PASSWORD\"",
},
wantToken: &testToken,
wantStdErr: "^\nLog in to some-upstream-name\n\n$",
wantToken: &testToken,
},
{
name: "with requested audience, session cache hit with valid token, but discovery fails",
Expand Down