Skip to content

Commit

Permalink
feat: nctl auth login in non-interactive env and without set token sh…
Browse files Browse the repository at this point in the history 
…ould not block

If you execute `nctl auth login` in a non-interactive environment (e.g. CI/CD)
and forget to set `--api-token`, the browser should not try to open - you
should see an error message instead.
  • Loading branch information
@pawelkuc
pawelkuc committed Dec 17, 2024
1 parent b2bb0f2 commit 9bb6cf3
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 52 deletions.
20 changes: 15 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,12 @@ 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 {
if !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
8 changes: 4 additions & 4 deletions get/application.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ func (cmd *applicationsCmd) Run(ctx context.Context, client *api.Client, get *Cm
fmt.Fprintf(defaultOut(cmd.out), "no application with basic auth enabled found\n")
return err
}
if printErr := printCredentials(creds, get, defaultOut(cmd.out)); printErr != nil {
if printErr := printCredentials(ctx, creds, get, defaultOut(cmd.out)); printErr != nil {
err = multierror.Append(err, printErr)
}
return err
}

if cmd.DNS {
return printDNSDetails(util.GatherDNSDetails(appList.Items), get, defaultOut(cmd.out))
return printDNSDetails(ctx, util.GatherDNSDetails(appList.Items), get, defaultOut(cmd.out))
}

switch get.Output {
Expand Down Expand Up @@ -101,7 +101,7 @@ func printApplication(apps []apps.Application, get *Cmd, out io.Writer, header b
return w.Flush()
}

func printCredentials(creds []appCredentials, get *Cmd, out io.Writer) error {
func printCredentials(ctx context.Context, creds []appCredentials, get *Cmd, out io.Writer) error {
if get.Output == yamlOut {
return format.PrettyPrintObjects(creds, format.PrintOpts{Out: out})
}
Expand Down Expand Up @@ -162,7 +162,7 @@ func join(list []string) string {
return strings.Join(list, ",")
}

func printDNSDetails(items []util.DNSDetail, get *Cmd, out io.Writer) error {
func printDNSDetails(ctx context.Context, items []util.DNSDetail, get *Cmd, out io.Writer) error {
if get.Output == yamlOut {
return format.PrettyPrintObjects(items, format.PrintOpts{Out: out})
}
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 9bb6cf3

Please sign in to comment.