Skip to content

Commit

Permalink
Merge pull request #197 from ninech/not-block-with-no-tty
Browse files Browse the repository at this point in the history 
auth login in non-interactive env should not block
  • Loading branch information
@pawelkuc
pawelkuc authored Dec 18, 2024
2 parents b2bb0f2 + efa74be commit a5ab996
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 48 deletions.
18 changes: 13 additions & 5 deletions auth/login.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package auth

import (
"context"
"errors"
"fmt"
"net/url"
"os"
Expand All @@ -17,13 +18,16 @@ import (
)

type LoginCmd struct {
APIURL string `help:"The URL of the Nine API" default:"https://nineapis.ch" env:"NCTL_API_URL" name:"api-url"`
APIToken string `help:"Use a static API token instead of using an OIDC login. You need to specify the --organization parameter as well." env:"NCTL_API_TOKEN"`
Organization string `help:"The name of your organization to use when providing an API token. This parameter is only used when providing a API token. This parameter needs to be set if you use --api-token." env:"NCTL_ORGANIZATION"`
IssuerURL string `help:"Issuer URL is the OIDC issuer URL of the API." default:"https://auth.nine.ch/auth/realms/pub"`
ClientID string `help:"Client ID is the OIDC client ID of the API." default:"nineapis.ch-f178254"`
APIURL string `help:"The URL of the Nine API" default:"https://nineapis.ch" env:"NCTL_API_URL" name:"api-url"`
APIToken string `help:"Use a static API token instead of using an OIDC login. You need to specify the --organization parameter as well." env:"NCTL_API_TOKEN"`
Organization string `help:"The name of your organization to use when providing an API token. This parameter is only used when providing a API token. This parameter needs to be set if you use --api-token." env:"NCTL_ORGANIZATION"`
IssuerURL string `help:"Issuer URL is the OIDC issuer URL of the API." default:"https://auth.nine.ch/auth/realms/pub"`
ClientID string `help:"Client ID is the OIDC client ID of the API." default:"nineapis.ch-f178254"`
ForceInteractiveEnvOverride bool `help:"Used for internal purposes only. Set to true to force interactive environment explicit override. Set to false to fall back to automatic interactivity detection." default:"false" hidden:""`
}

const ErrNonInteractiveEnvironmentEmptyToken = "a static API token is required in non-interactive environments"

func (l *LoginCmd) Run(ctx context.Context, command string, tk api.TokenGetter) error {
apiURL, err := url.Parse(l.APIURL)
if err != nil {
Expand Down Expand Up @@ -58,6 +62,10 @@ func (l *LoginCmd) Run(ctx context.Context, command string, tk api.TokenGetter)
return login(ctx, cfg, loadingRules.GetDefaultFilename(), userInfo.User, "", project(l.Organization))
}

if !l.ForceInteractiveEnvOverride && !format.IsInteractiveEnvironment(os.Stdout) {
return errors.New(ErrNonInteractiveEnvironmentEmptyToken)
}

usePKCE := true

token, err := tk.GetTokenString(ctx, l.IssuerURL, l.ClientID, usePKCE)
Expand Down
122 changes: 84 additions & 38 deletions auth/login_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"path"
"testing"

"github.com/ninech/nctl/api"
"github.com/ninech/nctl/internal/test"
"github.com/stretchr/testify/require"
"k8s.io/client-go/tools/clientcmd"
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
)
Expand All @@ -19,6 +21,16 @@ func (f *fakeTokenGetter) GetTokenString(ctx context.Context, issuerURL, clientI
return test.FakeJWTToken, nil
}

func checkErrorRequire(t *testing.T, err error, expectError bool, expectedErrMsg string) {
t.Helper()
if expectError {
require.Error(t, err, "expected an error but got none")
require.EqualError(t, err, expectedErrMsg, "unexpected error message")
} else {
require.NoError(t, err, "expected no error but got one")
}
}

func TestLoginCmd(t *testing.T) {
// write our "existing" kubeconfig to a temp kubeconfig
kubeconfig, err := os.CreateTemp("", "*-kubeconfig.yaml")
Expand All @@ -36,8 +48,9 @@ func TestLoginCmd(t *testing.T) {
apiHost := "api.example.org"
tk := &fakeTokenGetter{}
cmd := &LoginCmd{
APIURL: "https://" + apiHost,
IssuerURL: "https://auth.example.org",
APIURL: "https://" + apiHost,
IssuerURL: "https://auth.example.org",
ForceInteractiveEnvOverride: true,
}
if err := cmd.Run(context.Background(), "", tk); err != nil {
t.Fatal(err)
Expand All @@ -58,42 +71,74 @@ func TestLoginCmd(t *testing.T) {
}

func TestLoginStaticToken(t *testing.T) {
kubeconfig, err := os.CreateTemp("", "*-kubeconfig.yaml")
if err != nil {
log.Fatal(err)
}
defer os.Remove(kubeconfig.Name())

os.Setenv(clientcmd.RecommendedConfigPathEnvVar, kubeconfig.Name())

apiHost := "api.example.org"
token := test.FakeJWTToken

cmd := &LoginCmd{APIURL: "https://" + apiHost, APIToken: token, Organization: "test"}
tk := &fakeTokenGetter{}
if err := cmd.Run(context.Background(), "", tk); err != nil {
t.Fatal(err)
}

// read out the kubeconfig again to test the contents
b, err := io.ReadAll(kubeconfig)
if err != nil {
t.Fatal(err)
}

kc, err := clientcmd.Load(b)
if err != nil {
t.Fatal(err)
}

checkConfig(t, kc, 1, "")

if token != kc.AuthInfos[apiHost].Token {
t.Fatalf("expected token to be %s, got %s", token, kc.AuthInfos[apiHost].Token)
}

if kc.AuthInfos[apiHost].Exec != nil {
t.Fatalf("expected execConfig to be empty, got %v", kc.AuthInfos[apiHost].Exec)
tests := []struct {
name string
cmd *LoginCmd
tk api.TokenGetter
wantToken string
wantErr bool
wantErrMessage string
}{
{
name: "interactive environment with token",
cmd: &LoginCmd{APIURL: "https://" + apiHost, APIToken: test.FakeJWTToken, Organization: "test", ForceInteractiveEnvOverride: true},
tk: &fakeTokenGetter{},
wantToken: test.FakeJWTToken,
},
{
name: "non-interactive environment with token",
cmd: &LoginCmd{APIURL: "https://" + apiHost, APIToken: test.FakeJWTToken, Organization: "test", ForceInteractiveEnvOverride: false},
tk: &fakeTokenGetter{},
wantToken: test.FakeJWTToken,
},
{
name: "non-interactive environment with empty token",
cmd: &LoginCmd{APIURL: "https://" + apiHost, APIToken: "", Organization: "test", ForceInteractiveEnvOverride: false},
wantErr: true,
wantErrMessage: ErrNonInteractiveEnvironmentEmptyToken,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
kubeconfig, err := os.CreateTemp("", "*-kubeconfig.yaml")
if err != nil {
log.Fatal(err)
}
defer os.Remove(kubeconfig.Name())
os.Setenv(clientcmd.RecommendedConfigPathEnvVar, kubeconfig.Name())

err = tt.cmd.Run(context.Background(), "", tt.tk)
checkErrorRequire(t, err, tt.wantErr, tt.wantErrMessage)

if tt.wantErr {
return
}

// read out the kubeconfig again to test the contents
b, err := io.ReadAll(kubeconfig)
if err != nil {
t.Fatal(err)
}

kc, err := clientcmd.Load(b)
if err != nil {
t.Fatal(err)
}

checkConfig(t, kc, 1, "")

if tt.wantToken != kc.AuthInfos[apiHost].Token {
t.Fatalf("expected token to be %s, got %s", tt.wantToken, kc.AuthInfos[apiHost].Token)
}

if kc.AuthInfos[apiHost].Exec != nil {
t.Fatalf("expected execConfig to be empty, got %v", kc.AuthInfos[apiHost].Exec)
}
})
}
}

Expand All @@ -109,8 +154,9 @@ func TestLoginCmdWithoutExistingKubeconfig(t *testing.T) {

apiHost := "api.example.org"
cmd := &LoginCmd{
APIURL: "https://" + apiHost,
IssuerURL: "https://auth.example.org",
APIURL: "https://" + apiHost,
IssuerURL: "https://auth.example.org",
ForceInteractiveEnvOverride: true,
}
tk := &fakeTokenGetter{}
if err := cmd.Run(context.Background(), "", tk); err != nil {
Expand Down
14 changes: 9 additions & 5 deletions internal/format/print.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,7 @@ func getPrinter(out io.Writer) (printer.Printer, error) {
p := printer.Printer{
LineNumber: false,
}
f, isFile := out.(*os.File)
if !isFile {
return p, nil
}
if isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) {
if IsInteractiveEnvironment(out) {
p.Bool = printerProperty(&printer.Property{
Prefix: format(color.FgHiMagenta),
Suffix: format(color.Reset),
Expand All @@ -218,6 +214,14 @@ func getPrinter(out io.Writer) (printer.Printer, error) {
return p, nil
}

func IsInteractiveEnvironment(out io.Writer) bool {
f, isFile := out.(*os.File)
if !isFile {
return false
}
return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd())
}

// stripObj removes some fields which simply add clutter to the yaml output.
// The object should still be applyable afterwards as just defaults and
// computed fields get removed.
Expand Down

0 comments on commit a5ab996

Please sign in to comment.