Skip to content

Commit

Permalink
Add --oidc-redirect-url-hostname flag (#269)
Browse files Browse the repository at this point in the history
  • Loading branch information
int128 authored Apr 8, 2020
1 parent 7081984 commit 59b5f1b
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 40 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ Flags:
--listen-address strings Address to bind to the local server. If multiple addresses are given, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000])
--listen-port ints (Deprecated: use --listen-address)
--skip-open-browser If true, it does not open the browser on authentication
--oidc-redirect-url-hostname string Hostname of the redirect URL (default "localhost")
--oidc-auth-request-extra-params stringToString Extra query parameters to send with an authentication request (default [])
--username string If set, perform the resource owner password credentials grant
--password string If set, use the password instead of asking it
Expand Down Expand Up @@ -192,6 +193,12 @@ You can change the listening address.
- --listen-address=127.0.0.1:23456
```

You can change the hostname of redirect URI from the default value `localhost`.

```yaml
- --oidc-redirect-url-hostname=127.0.0.1
```

You can add extra parameters to the authentication request.

```yaml
Expand Down
52 changes: 27 additions & 25 deletions docs/standalone-mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,31 +99,33 @@ Available Commands:
version Print the version information
Flags:
--kubeconfig string Path to the kubeconfig file
--context string The name of the kubeconfig context to use
--user string The name of the kubeconfig user to use. Prior to --context
--certificate-authority string Path to a cert file for the certificate authority
--insecure-skip-tls-verify If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure
--grant-type string The authorization grant type to use. One of (auto|authcode|authcode-keyboard|password) (default "auto")
--listen-address strings Address to bind to the local server. If multiple addresses are given, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000])
--listen-port ints (Deprecated: use --listen-address)
--skip-open-browser If true, it does not open the browser on authentication
--username string If set, perform the resource owner password credentials grant
--password string If set, use the password instead of asking it
--add_dir_header If true, adds the file directory to the header
--alsologtostderr log to standard error as well as files
--log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0)
--log_dir string If non-empty, write log files in this directory
--log_file string If non-empty, use this log file
--log_file_max_size uint Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800)
--logtostderr log to standard error instead of files (default true)
--skip_headers If true, avoid header prefixes in the log messages
--skip_log_headers If true, avoid headers when opening log files
--stderrthreshold severity logs at or above this threshold go to stderr (default 2)
-v, --v Level number for the log level verbosity
--vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging
-h, --help help for main
--version version for main
--kubeconfig string Path to the kubeconfig file
--context string The name of the kubeconfig context to use
--user string The name of the kubeconfig user to use. Prior to --context
--certificate-authority string Path to a cert file for the certificate authority
--insecure-skip-tls-verify If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure
--grant-type string The authorization grant type to use. One of (auto|authcode|authcode-keyboard|password) (default "auto")
--listen-address strings Address to bind to the local server. If multiple addresses are given, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000])
--listen-port ints (Deprecated: use --listen-address)
--skip-open-browser If true, it does not open the browser on authentication
--oidc-redirect-url-hostname string Hostname of the redirect URL (default "localhost")
--oidc-auth-request-extra-params stringToString Extra query parameters to send with an authentication request (default [])
--username string If set, perform the resource owner password credentials grant
--password string If set, use the password instead of asking it
--add_dir_header If true, adds the file directory to the header
--alsologtostderr log to standard error as well as files
--log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0)
--log_dir string If non-empty, write log files in this directory
--log_file string If non-empty, use this log file
--log_file_max_size uint Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800)
--logtostderr log to standard error instead of files (default true)
--skip_headers If true, avoid header prefixes in the log messages
--skip_log_headers If true, avoid headers when opening log files
--stderrthreshold severity logs at or above this threshold go to stderr (default 2)
-v, --v Level number for the log level verbosity
--vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging
-h, --help help for kubelogin
--version version for kubelogin
```

### Kubeconfig
Expand Down
32 changes: 28 additions & 4 deletions integration_test/credetial_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func testCredentialPlugin(t *testing.T, tc credentialPluginTestCase) {
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), tc.Keys)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "openid", nil, &idToken)
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://localhost:", nil, &idToken)
writerMock := newCredentialPluginWriterMock(t, ctrl, &idToken)
browserMock := newBrowserMock(ctx, t, ctrl, tc.Keys)

Expand Down Expand Up @@ -214,7 +214,7 @@ func testCredentialPlugin(t *testing.T, tc credentialPluginTestCase) {
validIDToken := newIDToken(t, serverURL, "YOUR_NONCE", tokenExpiryFuture)
expiredIDToken := newIDToken(t, serverURL, "YOUR_NONCE", tokenExpiryPast)

setupAuthCodeFlow(t, provider, serverURL, "openid", nil, &validIDToken)
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://localhost:", nil, &validIDToken)
provider.EXPECT().Refresh("EXPIRED_REFRESH_TOKEN").
Return(nil, &idp.ErrorResponse{Code: "invalid_request", Description: "token has expired"}).
MaxTimes(2) // package oauth2 will retry refreshing the token
Expand Down Expand Up @@ -249,7 +249,7 @@ func testCredentialPlugin(t *testing.T, tc credentialPluginTestCase) {
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), tc.Keys)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "email profile openid", nil, &idToken)
setupAuthCodeFlow(t, provider, serverURL, "email profile openid", "http://localhost:", nil, &idToken)
writerMock := newCredentialPluginWriterMock(t, ctrl, &idToken)
browserMock := newBrowserMock(ctx, t, ctrl, tc.Keys)

Expand All @@ -263,6 +263,30 @@ func testCredentialPlugin(t *testing.T, tc credentialPluginTestCase) {
runGetTokenCmd(t, ctx, browserMock, writerMock, args)
})

t.Run("RedirectURLHostname", func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.TODO(), timeout)
defer cancel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

provider := mock_idp.NewMockProvider(ctrl)
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), tc.Keys)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://127.0.0.1:", nil, &idToken)
writerMock := newCredentialPluginWriterMock(t, ctrl, &idToken)
browserMock := newBrowserMock(ctx, t, ctrl, tc.Keys)

args := []string{
"--oidc-issuer-url", serverURL,
"--oidc-client-id", "kubernetes",
"--oidc-redirect-url-hostname", "127.0.0.1",
}
args = append(args, tc.ExtraArgs...)
runGetTokenCmd(t, ctx, browserMock, writerMock, args)
})

t.Run("ExtraParams", func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.TODO(), timeout)
Expand All @@ -274,7 +298,7 @@ func testCredentialPlugin(t *testing.T, tc credentialPluginTestCase) {
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), tc.Keys)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "openid", map[string]string{
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://localhost:", map[string]string{
"ttl": "86400",
"reauth": "false",
}, &idToken)
Expand Down
6 changes: 5 additions & 1 deletion integration_test/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration_test
import (
"context"
"net/http"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -33,7 +34,7 @@ func newIDToken(t *testing.T, issuer, nonce string, expiry time.Time) string {
})
}

func setupAuthCodeFlow(t *testing.T, provider *mock_idp.MockProvider, serverURL, scope string, extraParams map[string]string, idToken *string) {
func setupAuthCodeFlow(t *testing.T, provider *mock_idp.MockProvider, serverURL, scope, redirectURIPrefix string, extraParams map[string]string, idToken *string) {
var nonce string
provider.EXPECT().Discovery().Return(idp.NewDiscoveryResponse(serverURL))
provider.EXPECT().GetCertificates().Return(idp.NewCertificatesResponse(jwt.PrivateKey))
Expand All @@ -42,6 +43,9 @@ func setupAuthCodeFlow(t *testing.T, provider *mock_idp.MockProvider, serverURL,
if req.Scope != scope {
t.Errorf("scope wants `%s` but was `%s`", scope, req.Scope)
}
if !strings.HasPrefix(req.RedirectURI, redirectURIPrefix) {
t.Errorf("redirectURI wants prefix `%s` but was `%s`", redirectURIPrefix, req.RedirectURI)
}
for k, v := range extraParams {
got := req.RawQuery.Get(k)
if got != v {
Expand Down
8 changes: 4 additions & 4 deletions integration_test/standalone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func testStandalone(t *testing.T, idpTLS keys.Keys) {
defer server.Shutdown(t, ctx)
browserMock := newBrowserMock(ctx, t, ctrl, idpTLS)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "openid", nil, &idToken)
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://localhost:", nil, &idToken)
kubeConfigFilename := kubeconfig.Create(t, &kubeconfig.Values{
Issuer: serverURL,
IDPCertificateAuthority: idpTLS.CACertPath,
Expand Down Expand Up @@ -175,7 +175,7 @@ func testStandalone(t *testing.T, idpTLS keys.Keys) {
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), idpTLS)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "openid", nil, &idToken)
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://localhost:", nil, &idToken)
provider.EXPECT().Refresh("EXPIRED_REFRESH_TOKEN").
Return(nil, &idp.ErrorResponse{Code: "invalid_request", Description: "token has expired"}).
MaxTimes(2) // package oauth2 will retry refreshing the token
Expand Down Expand Up @@ -210,7 +210,7 @@ func testStandalone(t *testing.T, idpTLS keys.Keys) {
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), idpTLS)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "openid", nil, &idToken)
setupAuthCodeFlow(t, provider, serverURL, "openid", "http://localhost:", nil, &idToken)
browserMock := newBrowserMock(ctx, t, ctrl, idpTLS)

kubeConfigFilename := kubeconfig.Create(t, &kubeconfig.Values{
Expand Down Expand Up @@ -242,7 +242,7 @@ func testStandalone(t *testing.T, idpTLS keys.Keys) {
serverURL, server := localserver.Start(t, idp.NewHandler(t, provider), idpTLS)
defer server.Shutdown(t, ctx)
var idToken string
setupAuthCodeFlow(t, provider, serverURL, "profile groups openid", nil, &idToken)
setupAuthCodeFlow(t, provider, serverURL, "profile groups openid", "http://localhost:", nil, &idToken)
browserMock := newBrowserMock(ctx, t, ctrl, idpTLS)

kubeConfigFilename := kubeconfig.Create(t, &kubeconfig.Values{
Expand Down
18 changes: 12 additions & 6 deletions pkg/adaptors/cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ func TestCmd_Run(t *testing.T) {
in: standalone.Input{
GrantOptionSet: authentication.GrantOptionSet{
AuthCodeOption: &authentication.AuthCodeOption{
BindAddress: defaultListenAddress,
BindAddress: defaultListenAddress,
RedirectURLHostname: "localhost",
},
},
},
Expand All @@ -41,7 +42,8 @@ func TestCmd_Run(t *testing.T) {
in: standalone.Input{
GrantOptionSet: authentication.GrantOptionSet{
AuthCodeOption: &authentication.AuthCodeOption{
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
RedirectURLHostname: "localhost",
},
},
},
Expand All @@ -57,7 +59,8 @@ func TestCmd_Run(t *testing.T) {
in: standalone.Input{
GrantOptionSet: authentication.GrantOptionSet{
AuthCodeOption: &authentication.AuthCodeOption{
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
RedirectURLHostname: "localhost",
},
},
},
Expand Down Expand Up @@ -85,8 +88,9 @@ func TestCmd_Run(t *testing.T) {
SkipTLSVerify: true,
GrantOptionSet: authentication.GrantOptionSet{
AuthCodeOption: &authentication.AuthCodeOption{
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
SkipOpenBrowser: true,
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
SkipOpenBrowser: true,
RedirectURLHostname: "localhost",
},
},
},
Expand Down Expand Up @@ -191,7 +195,8 @@ func TestCmd_Run(t *testing.T) {
ClientID: "YOUR_CLIENT_ID",
GrantOptionSet: authentication.GrantOptionSet{
AuthCodeOption: &authentication.AuthCodeOption{
BindAddress: []string{"127.0.0.1:8000", "127.0.0.1:18000"},
BindAddress: []string{"127.0.0.1:8000", "127.0.0.1:18000"},
RedirectURLHostname: "localhost",
},
},
},
Expand Down Expand Up @@ -230,6 +235,7 @@ func TestCmd_Run(t *testing.T) {
AuthCodeOption: &authentication.AuthCodeOption{
BindAddress: []string{"127.0.0.1:10080", "127.0.0.1:20080"},
SkipOpenBrowser: true,
RedirectURLHostname: "localhost",
AuthRequestExtraParams: map[string]string{"ttl": "86400", "reauth": "true"},
},
},
Expand Down
3 changes: 3 additions & 0 deletions pkg/adaptors/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type authenticationOptions struct {
ListenAddress []string
ListenPort []int // deprecated
SkipOpenBrowser bool
RedirectURLHostname string
AuthRequestExtraParams map[string]string
Username string
Password string
Expand Down Expand Up @@ -81,6 +82,7 @@ func (o *authenticationOptions) register(f *pflag.FlagSet) {
//TODO: remove the deprecated flag
f.IntSliceVar(&o.ListenPort, "listen-port", nil, "(Deprecated: use --listen-address)")
f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "If true, it does not open the browser on authentication")
f.StringVar(&o.RedirectURLHostname, "oidc-redirect-url-hostname", "localhost", "Hostname of the redirect URL")
f.StringToStringVar(&o.AuthRequestExtraParams, "oidc-auth-request-extra-params", nil, "Extra query parameters to send with an authentication request")
f.StringVar(&o.Username, "username", "", "If set, perform the resource owner password credentials grant")
f.StringVar(&o.Password, "password", "", "If set, use the password instead of asking it")
Expand All @@ -92,6 +94,7 @@ func (o *authenticationOptions) grantOptionSet() (s authentication.GrantOptionSe
s.AuthCodeOption = &authentication.AuthCodeOption{
BindAddress: o.determineListenAddress(),
SkipOpenBrowser: o.SkipOpenBrowser,
RedirectURLHostname: o.RedirectURLHostname,
AuthRequestExtraParams: o.AuthRequestExtraParams,
}
case o.GrantType == "authcode-keyboard":
Expand Down
2 changes: 2 additions & 0 deletions pkg/adaptors/oidcclient/oidcclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type GetTokenByAuthCodeInput struct {
CodeChallenge string
CodeChallengeMethod string
CodeVerifier string
RedirectURLHostname string
AuthRequestExtraParams map[string]string
}

Expand Down Expand Up @@ -93,6 +94,7 @@ func (c *client) GetTokenByAuthCode(ctx context.Context, in GetTokenByAuthCodeIn
},
LocalServerBindAddress: in.BindAddress,
LocalServerReadyChan: localServerReadyChan,
RedirectURLHostname: in.RedirectURLHostname,
}
for key, value := range in.AuthRequestExtraParams {
config.AuthCodeOptions = append(config.AuthCodeOptions, oauth2.SetAuthURLParam(key, value))
Expand Down
1 change: 1 addition & 0 deletions pkg/usecases/authentication/authcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func (u *AuthCode) Do(ctx context.Context, o *AuthCodeOption, client oidcclient.
CodeChallenge: p.CodeChallenge,
CodeChallengeMethod: p.CodeChallengeMethod,
CodeVerifier: p.CodeVerifier,
RedirectURLHostname: o.RedirectURLHostname,
AuthRequestExtraParams: o.AuthRequestExtraParams,
}
readyChan := make(chan string, 1)
Expand Down
4 changes: 4 additions & 0 deletions pkg/usecases/authentication/authcode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func TestAuthCode_Do(t *testing.T) {
o := &AuthCodeOption{
BindAddress: []string{"127.0.0.1:8000"},
SkipOpenBrowser: true,
RedirectURLHostname: "localhost",
AuthRequestExtraParams: map[string]string{"ttl": "86400", "reauth": "true"},
}
mockOIDCClient := mock_oidcclient.NewMockInterface(ctrl)
Expand All @@ -39,6 +40,9 @@ func TestAuthCode_Do(t *testing.T) {
if diff := cmp.Diff(o.BindAddress, in.BindAddress); diff != "" {
t.Errorf("BindAddress mismatch (-want +got):\n%s", diff)
}
if diff := cmp.Diff(o.RedirectURLHostname, in.RedirectURLHostname); diff != "" {
t.Errorf("RedirectURLHostname mismatch (-want +got):\n%s", diff)
}
if diff := cmp.Diff(o.AuthRequestExtraParams, in.AuthRequestExtraParams); diff != "" {
t.Errorf("AuthRequestExtraParams mismatch (-want +got):\n%s", diff)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/usecases/authentication/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type GrantOptionSet struct {
type AuthCodeOption struct {
SkipOpenBrowser bool
BindAddress []string
RedirectURLHostname string
AuthRequestExtraParams map[string]string
}

Expand Down

0 comments on commit 59b5f1b

Please sign in to comment.