Skip to content

Commit 9ed9d5e

Browse files
rsdmikersdmike
authored
refactor: enhance validation for authenticating with console (#1235)
1 parent c2737ad commit 9ed9d5e

5 files changed

Lines changed: 233 additions & 7 deletions

File tree

‎internal/cli/cli.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import (
2525
"golang.org/x/term"
2626
)
2727

28+
const configFilePath = "config.yaml"
29+
2830
// Global flags that apply to all commands
2931
type Globals struct {
3032
// Configuration handling
@@ -109,7 +111,7 @@ func Parse(args []string, amtCommand amt.Interface) (*kong.Context, *CLI, error)
109111
kong.UsageOnError(),
110112
kong.DefaultEnvars("RPC"),
111113
kong.ConfigureHelp(helpOpts),
112-
kong.Configuration(kongyaml.Loader, "config.yaml"),
114+
kong.Configuration(kongyaml.Loader, configFilePath),
113115
kong.BindToProvider(func() amt.Interface { return amtCommand }),
114116
}
115117

@@ -127,6 +129,12 @@ func Parse(args []string, amtCommand amt.Interface) (*kong.Context, *CLI, error)
127129
}
128130

129131
ctx, perr := parser.Parse(parseArgs)
132+
133+
// Log config file presence after parsing (logging is configured by AfterApply at this point)
134+
if _, statErr := os.Stat(configFilePath); statErr == nil {
135+
log.Infof("Using configuration file: %s (flag values may originate from this file)", configFilePath)
136+
}
137+
130138
if perr == nil {
131139
return ctx, &cli, nil
132140
}

‎internal/commands/activate/activate.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func (cmd *ActivateCmd) runHttpProfileFullflow(ctx *commands.Context) error {
307307

308308
cfg, err := fetcher.FetchProfile()
309309
if err != nil {
310-
return fmt.Errorf("failed to fetch profile: %w", err)
310+
return err
311311
}
312312

313313
// Resolve AMT/MEBx/MPS passwords — generate random ones when the profile requests it.

‎internal/commands/auth.go‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,41 @@ import (
1313
"github.com/sirupsen/logrus"
1414
)
1515

16-
// ServerAuthFlags provides common auth options for server communications
17-
// Either AuthToken (Bearer) OR both AuthUsername and AuthPassword (Basic) should be supplied.
16+
// ServerAuthFlags provides common auth options for server communications.
17+
// When AuthEndpoint is set, either AuthToken (Bearer) OR both AuthUsername
18+
// and AuthPassword (Basic) must be supplied.
1819
type ServerAuthFlags struct {
1920
AuthToken string `help:"Bearer token for server authentication" name:"auth-token" env:"AUTH_TOKEN"`
2021
AuthUsername string `help:"Username for basic auth (used when no token)" name:"auth-username" env:"AUTH_USERNAME"`
2122
AuthPassword string `help:"Password for basic auth (used when no token)" name:"auth-password" env:"AUTH_PASSWORD"`
22-
// Optional endpoint for exchanging credentials for a token (primarily used when fetching HTTP profiles)
23-
AuthEndpoint string `help:"The endpoint to call to fetch a token. Assumes the same host as the profile URL unless an absolute URL is provided; defaults to the Console path /api/v1/authorize." name:"auth-endpoint" default:"/api/v1/authorize"`
23+
AuthEndpoint string `help:"Token exchange endpoint. Requires --auth-token or --auth-username/--auth-password. Resolved relative to the profile URL host unless absolute." name:"auth-endpoint" env:"AUTH_ENDPOINT"`
24+
}
25+
26+
// Validate implements kong.Validatable.
27+
// - auth-username and auth-password must always be provided together.
28+
// - When auth-endpoint is set, either auth-token or (auth-username + auth-password) is required.
29+
func (a *ServerAuthFlags) Validate() error {
30+
if (a.AuthUsername != "") != (a.AuthPassword != "") {
31+
if a.AuthUsername != "" {
32+
return fmt.Errorf("--auth-username requires --auth-password")
33+
}
34+
35+
return fmt.Errorf("--auth-password requires --auth-username")
36+
}
37+
38+
if a.AuthEndpoint == "" {
39+
return nil
40+
}
41+
42+
if a.AuthToken != "" {
43+
return nil
44+
}
45+
46+
if a.AuthUsername != "" && a.AuthPassword != "" {
47+
return nil
48+
}
49+
50+
return fmt.Errorf("--auth-endpoint requires --auth-token or both --auth-username and --auth-password")
2451
}
2552

2653
// ValidateRequired enforces that some form of auth is present when required.

‎internal/commands/auth_test.go‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/*********************************************************************
2+
* Copyright (c) Intel Corporation 2024
3+
* SPDX-License-Identifier: Apache-2.0
4+
**********************************************************************/
5+
6+
package commands
7+
8+
import (
9+
"context"
10+
"net/http"
11+
"testing"
12+
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestServerAuthFlags_Validate(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
flags ServerAuthFlags
21+
wantErr string
22+
}{
23+
{
24+
name: "no endpoint — no validation",
25+
flags: ServerAuthFlags{},
26+
},
27+
{
28+
name: "endpoint with token — ok",
29+
flags: ServerAuthFlags{
30+
AuthEndpoint: "/api/v1/authorize",
31+
AuthToken: "tok",
32+
},
33+
},
34+
{
35+
name: "endpoint with username and password — ok",
36+
flags: ServerAuthFlags{
37+
AuthEndpoint: "/api/v1/authorize",
38+
AuthUsername: "user",
39+
AuthPassword: "pass",
40+
},
41+
},
42+
{
43+
name: "endpoint with no credentials — error",
44+
flags: ServerAuthFlags{
45+
AuthEndpoint: "/api/v1/authorize",
46+
},
47+
wantErr: "--auth-endpoint requires --auth-token or both --auth-username and --auth-password",
48+
},
49+
{
50+
name: "endpoint with username only — error",
51+
flags: ServerAuthFlags{
52+
AuthEndpoint: "/api/v1/authorize",
53+
AuthUsername: "user",
54+
},
55+
wantErr: "--auth-username requires --auth-password",
56+
},
57+
{
58+
name: "endpoint with password only — error",
59+
flags: ServerAuthFlags{
60+
AuthEndpoint: "/api/v1/authorize",
61+
AuthPassword: "pass",
62+
},
63+
wantErr: "--auth-password requires --auth-username",
64+
},
65+
{
66+
name: "username only without endpoint — error",
67+
flags: ServerAuthFlags{
68+
AuthUsername: "user",
69+
},
70+
wantErr: "--auth-username requires --auth-password",
71+
},
72+
{
73+
name: "password only without endpoint — error",
74+
flags: ServerAuthFlags{
75+
AuthPassword: "pass",
76+
},
77+
wantErr: "--auth-password requires --auth-username",
78+
},
79+
{
80+
name: "endpoint with token and username/password — token wins, ok",
81+
flags: ServerAuthFlags{
82+
AuthEndpoint: "/api/v1/authorize",
83+
AuthToken: "tok",
84+
AuthUsername: "user",
85+
AuthPassword: "pass",
86+
},
87+
},
88+
}
89+
90+
for _, tt := range tests {
91+
t.Run(tt.name, func(t *testing.T) {
92+
err := tt.flags.Validate()
93+
if tt.wantErr != "" {
94+
require.Error(t, err)
95+
assert.Contains(t, err.Error(), tt.wantErr)
96+
} else {
97+
assert.NoError(t, err)
98+
}
99+
})
100+
}
101+
}
102+
103+
func TestServerAuthFlags_ValidateRequired(t *testing.T) {
104+
tests := []struct {
105+
name string
106+
flags ServerAuthFlags
107+
required bool
108+
wantErr bool
109+
}{
110+
{
111+
name: "not required — always ok",
112+
flags: ServerAuthFlags{},
113+
required: false,
114+
},
115+
{
116+
name: "required with token",
117+
flags: ServerAuthFlags{AuthToken: "tok"},
118+
required: true,
119+
},
120+
{
121+
name: "required with username and password",
122+
flags: ServerAuthFlags{AuthUsername: "user", AuthPassword: "pass"},
123+
required: true,
124+
},
125+
{
126+
name: "required with nothing",
127+
flags: ServerAuthFlags{},
128+
required: true,
129+
wantErr: true,
130+
},
131+
{
132+
name: "required with username only",
133+
flags: ServerAuthFlags{AuthUsername: "user"},
134+
required: true,
135+
wantErr: true,
136+
},
137+
}
138+
139+
for _, tt := range tests {
140+
t.Run(tt.name, func(t *testing.T) {
141+
err := tt.flags.ValidateRequired(tt.required)
142+
if tt.wantErr {
143+
assert.Error(t, err)
144+
} else {
145+
assert.NoError(t, err)
146+
}
147+
})
148+
}
149+
}
150+
151+
func TestServerAuthFlags_ApplyToRequest(t *testing.T) {
152+
tests := []struct {
153+
name string
154+
flags ServerAuthFlags
155+
wantHeader string
156+
}{
157+
{
158+
name: "token sets bearer",
159+
flags: ServerAuthFlags{AuthToken: "my-token"},
160+
wantHeader: "Bearer my-token",
161+
},
162+
{
163+
name: "username/password sets basic",
164+
flags: ServerAuthFlags{AuthUsername: "user", AuthPassword: "pass"},
165+
wantHeader: "Basic dXNlcjpwYXNz",
166+
},
167+
{
168+
name: "token takes precedence over basic",
169+
flags: ServerAuthFlags{AuthToken: "tok", AuthUsername: "user", AuthPassword: "pass"},
170+
wantHeader: "Bearer tok",
171+
},
172+
{
173+
name: "no credentials — no header",
174+
flags: ServerAuthFlags{},
175+
wantHeader: "",
176+
},
177+
}
178+
179+
for _, tt := range tests {
180+
t.Run(tt.name, func(t *testing.T) {
181+
req, _ := http.NewRequestWithContext(context.Background(), "GET", "http://example.com", nil)
182+
tt.flags.ApplyToRequest(req)
183+
184+
assert.Equal(t, tt.wantHeader, req.Header.Get("Authorization"))
185+
})
186+
}
187+
}

‎internal/profile/fetcher.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,11 @@ func (f *ProfileFetcher) fetchData(u, token string) ([]byte, error) {
198198
defer resp.Body.Close()
199199

200200
if resp.StatusCode == http.StatusUnauthorized {
201-
return nil, fmt.Errorf("unauthorized: authentication required or token invalid")
201+
if token == "" {
202+
return nil, fmt.Errorf("unauthorized: no credentials provided — use --auth-token or --auth-username/--auth-password")
203+
}
204+
205+
return nil, fmt.Errorf("unauthorized: server rejected the bearer token")
202206
}
203207

204208
if resp.StatusCode != http.StatusOK {

0 commit comments

Comments
 (0)