Add KeyCloak realm as a CLI config

Author: thossain-nv

docs/configuration/tenant-management-keycloak.md

@@ -162,8 +162,7 @@ bundled `ncx-service` client uses. Requires `serviceAccount: true` in the NICo c
 ```
 
 **Option B, a human user.** The bundled realm ships no human users, so create one, set
-`oidc_id`, and assign the role. Interactive login goes through the `nico-rest` client,
-which already has the standard flow enabled.
+`oidc_id`, and assign the role.
 
 ```bash
 /opt/keycloak/bin/kcadm.sh create users -r nico \
@@ -184,8 +183,48 @@ which already has the standard flow enabled.
 Any value for `oidc_id` works as long as it is unique within the realm and stable for
 the life of the user. Changing it later makes NICo treat the login as a new user.
 
+The doubled `u` in `--uusername` is not a typo. `kcadm.sh add-roles` prefixes each
+option with the target type, so `--uusername` names a user and `--cclientid` names a
+client, while `set-password` in the previous command takes a plain `--username`.
+
 Exit the pod shell when finished.
 
+### How a human user signs in
+
+There is no browser step. `nicocli` implements the OAuth password grant, the client
+credentials grant, and refresh-token renewal, and has no authorization-code or device-code
+flow, so Keycloak's login page is never shown. `nicocli login` collects the username and
+password (prompting when they are not supplied) and exchanges them at the realm's token
+endpoint directly. This requires `directAccessGrantsEnabled` on the client, which the
+bundled `nico-rest` client has.
+
+Two `nicocli` flag defaults do not match a `setup.sh` deployment and have to be overridden:
+`--keycloak-realm` defaults to `nico-dev` and `--client-id` defaults to `nico-api`, both of
+which are Kustomize dev values.
+
+```bash
+nicocli login \
+  --keycloak-url http://keycloak.nico-rest:8082 \
+  --keycloak-realm nico \
+  --client-id nico-rest \
+  --client-secret nico-local-secret \
+  --username tenant-admin@acme-corp.example
+```
+
+`--keycloak-url` has to be the host in `externalBaseURL`, because that is what the issuer
+is validated against. With the in-cluster default that name does not resolve from a
+workstation, so interactive sign-in from outside the cluster needs one of:
+
+- Set `externalBaseURL` to an externally resolvable hostname and expose Keycloak through an
+  ingress, restricted to the endpoints listed in
+  [Authentication and Authorization](https://docs.nvidia.com/infra-controller/rest-api-reference/authentication-and-authorization).
+  This is the production answer.
+- Or, for evaluation only, port-forward Keycloak and map the in-cluster name to `127.0.0.1`
+  in `/etc/hosts`, so the issuer in the minted token still matches.
+
+The Keycloak admin console is for realm administration, not for Tenant sign-in. Human
+Tenants never need an account in it.
+
 ### Verify the token maps to the org
 
 Request a token from inside the cluster. For Option A:

rest-api/cli/README.md

@@ -174,6 +174,7 @@ The scalar fields listed below can also be set via a `NICO_*` environment variab
 | `NICO_TOKEN_COMMAND` | `auth.token_command` | Shell command that prints a bearer token |
 | `NICO_AUTH_SCRIPT` | `auth.token_command` | Alias of `NICO_TOKEN_COMMAND` (canonical name wins when both set) |
 | `NICO_TOKEN_URL` | `auth.oidc.token_url` | |
+| `NICO_KEYCLOAK_REALM` | `auth.oidc.realm` | Used only when the token endpoint is built from `--keycloak-url` |
 | `NICO_CLIENT_ID` | `auth.oidc.client_id` | |
 | `NICO_CLIENT_SECRET` | `auth.oidc.client_secret` | |
 | `NICO_OIDC_USERNAME` | `auth.oidc.username` | |
@@ -185,7 +186,14 @@ The scalar fields listed below can also be set via a `NICO_*` environment variab
 | `NICO_AUTHN_URL` | `auth.api_key.authn_url` | Required for legacy NGC keys; ignored for `nvapi-` bearer keys |
 | `NICO_API_KEY_TOKEN` | `auth.api_key.token` | Persisted token after NGC exchange |
 
-`NICO_KEYCLOAK_URL` and `NICO_KEYCLOAK_REALM` do not map to a single config field; they feed the login command and construct the OIDC `token_url` at login time.
+`NICO_KEYCLOAK_URL` does not map to a config field of its own; it feeds the login command and
+constructs the OIDC `token_url`, which is what gets persisted.
+
+`--keycloak-realm` and `--client-id` both carry built-in defaults (`nico-dev` and `nico-api`)
+that match the Kustomize development realm, not the realm `helm-prereqs/setup.sh` installs.
+Set `auth.oidc.realm` and `auth.oidc.client_id` for your deployment rather than relying on
+them. When a login fails, the error names the token endpoint it contacted and any value that
+came from a built-in default.
 
 Client-credentials configurations can also set `auth.oidc.scopes` as a YAML list,
 `auth.oidc.token_parameters` as a map of additional non-secret form parameters,
@@ -307,7 +315,7 @@ Commands follow `nicocli <resource> [sub-resource] <action> [args] [flags]`.
 
 Nested API paths appear as sub-resource groups:
 
-```
+```bash
 nicocli allocation list
 nicocli allocation constraint list
 nicocli allocation constraint create <allocationId>
@@ -330,7 +338,7 @@ nicocli completion fish > ~/.config/fish/completions/nicocli.fish
 
 Each environment (local dev, staging, prod) gets its own config file in `~/.nico/`:
 
-```
+```text
 ~/.nico/config.yaml           # default (local dev)
 ~/.nico/config.staging.yaml   # staging
 ~/.nico/config.prod.yaml      # production

rest-api/cli/pkg/auth.go

@@ -376,14 +376,64 @@ func extractNGCToken(body []byte) string {
 	return resp.AccessToken
 }
 
+// resolveOIDCRealm returns the Keycloak realm used to build the token endpoint from
+// --keycloak-url, and whether it came from the flag's built-in default rather than from
+// the command line or config. An explicit flag wins over config, matching client-id.
+func resolveOIDCRealm(c *cli.Context, cfg *ConfigFile) (realm string, fromDefault bool) {
+	if cliFlagExplicitlySet(c, "keycloak-realm") {
+		return c.String("keycloak-realm"), false
+	}
+	if cfg.Auth.OIDC != nil && cfg.Auth.OIDC.Realm != "" {
+		return cfg.Auth.OIDC.Realm, false
+	}
+	return c.String("keycloak-realm"), true
+}
+
+// resolveOIDCClientID returns the OAuth client ID, and whether it came from the flag's
+// built-in default rather than from the command line or config.
+func resolveOIDCClientID(c *cli.Context, cfg *ConfigFile) (clientID string, fromDefault bool) {
+	if cliFlagExplicitlySet(c, "client-id") {
+		return c.String("client-id"), false
+	}
+	if cfg.Auth.OIDC != nil && cfg.Auth.OIDC.ClientID != "" {
+		return cfg.Auth.OIDC.ClientID, false
+	}
+	return c.String("client-id"), true
+}
+
+// loginFailureHint names the token endpoint that was actually contacted, plus any values
+// that came from a built-in default. A realm or client that does not exist in the target
+// Keycloak fails with a 404 or invalid_client that identifies neither, so without this
+// the built-in defaults are invisible in the error.
+func loginFailureHint(tokenURL string, defaulted []string) string {
+	hint := "\n  token endpoint: " + tokenURL
+	if len(defaulted) > 0 {
+		hint += "\n  using built-in default " + strings.Join(defaulted, " and ")
+		hint += "\n  pass the flag explicitly, or set auth.oidc in " + ConfigPath()
+	}
+	return hint
+}
+
 func loginWithOIDCCmd(c *cli.Context, cfg *ConfigFile) error {
 	tokenURL := c.String("token-url")
 	if tokenURL == "" && cfg.Auth.OIDC != nil {
 		tokenURL = cfg.Auth.OIDC.TokenURL
 	}
+
+	// Values that fell back to a built-in default, recorded so a failed login can name
+	// them. Only populated where the value was actually used: a realm supplied through
+	// --token-url never goes through resolveOIDCRealm.
+	var defaulted []string
+	var resolvedRealm string
+
 	if tokenURL == "" {
-		if keycloakURL := c.String("keycloak-url"); keycloakURL != "" {
-			realm := c.String("keycloak-realm")
+		keycloakURL := c.String("keycloak-url")
+		if keycloakURL != "" {
+			realm, realmFromDefault := resolveOIDCRealm(c, cfg)
+			if realmFromDefault {
+				defaulted = append(defaulted, "--keycloak-realm="+realm)
+			}
+			resolvedRealm = realm
 			tokenURL = fmt.Sprintf("%s/realms/%s/protocol/openid-connect/token",
 				strings.TrimRight(keycloakURL, "/"), realm)
 		}
@@ -392,9 +442,9 @@ func loginWithOIDCCmd(c *cli.Context, cfg *ConfigFile) error {
 		return fmt.Errorf("--token-url or --keycloak-url is required (or set auth.oidc.token_url in config)")
 	}
 
-	clientID := c.String("client-id")
-	if cfg.Auth.OIDC != nil && cfg.Auth.OIDC.ClientID != "" && !cliFlagExplicitlySet(c, "client-id") {
-		clientID = cfg.Auth.OIDC.ClientID
+	clientID, clientIDFromDefault := resolveOIDCClientID(c, cfg)
+	if clientIDFromDefault {
+		defaulted = append(defaulted, "--client-id="+clientID)
 	}
 
 	clientSecret := c.String("client-secret")
@@ -443,7 +493,7 @@ func loginWithOIDCCmd(c *cli.Context, cfg *ConfigFile) error {
 		tokenResp, err = passwordGrant(tokenURL, clientID, clientSecret, username, password)
 	}
 	if err != nil {
-		return err
+		return fmt.Errorf("%w%s", err, loginFailureHint(tokenURL, defaulted))
 	}
 
 	if cfg.Auth.OIDC == nil {
@@ -455,6 +505,11 @@ func loginWithOIDCCmd(c *cli.Context, cfg *ConfigFile) error {
 	cfg.Auth.OIDC.TokenURL = tokenURL
 	cfg.Auth.OIDC.ClientID = clientID
 	cfg.Auth.OIDC.ClientSecret = clientSecret
+	// Only persist a realm that was used to build tokenURL; a realm is meaningless
+	// against a token endpoint supplied directly.
+	if resolvedRealm != "" {
+		cfg.Auth.OIDC.Realm = resolvedRealm
+	}
 
 	if err := SaveConfig(cfg); err != nil {
 		return fmt.Errorf("saving config: %w", err)

rest-api/cli/pkg/auth_test.go

@@ -115,6 +115,10 @@ func TestLoginWithOIDCCmd(t *testing.T) {
 		name         string
 		clientIDArgs []string
 		wantClientID string
+		// rejectStatus, when non-zero, makes the token endpoint fail so the case can
+		// assert what a failed login reports back to the user.
+		rejectStatus int
+		wantErr      string
 	}{
 		{
 			name:         "preserves configured client ID",
@@ -125,6 +129,12 @@ func TestLoginWithOIDCCmd(t *testing.T) {
 			clientIDArgs: []string{"--client-id", "override-id"},
 			wantClientID: "override-id",
 		},
+		{
+			name:         "failed login names the token endpoint",
+			wantClientID: "client-id",
+			rejectStatus: http.StatusNotFound,
+			wantErr:      "token endpoint: ",
+		},
 	}
 
 	for _, tt := range tests {
@@ -137,6 +147,10 @@ func TestLoginWithOIDCCmd(t *testing.T) {
 				require.True(t, ok)
 				require.Equal(t, tt.wantClientID, clientID)
 				require.Equal(t, "client-secret", clientSecret)
+				if tt.rejectStatus != 0 {
+					w.WriteHeader(tt.rejectStatus)
+					return
+				}
 				w.Header().Set("Content-Type", "application/json")
 				_, _ = w.Write([]byte(`{"access_token":"new-token","expires_in":3600}`))
 			}))
@@ -159,11 +173,154 @@ func TestLoginWithOIDCCmd(t *testing.T) {
 			args := append([]string{"nicocli", "--token-url", server.URL}, tt.clientIDArgs...)
 			args = append(args, "login")
 			withArgs(t, args...)
-			require.NoError(t, app.Run(os.Args))
+
+			runErr := app.Run(os.Args)
+			if tt.wantErr != "" {
+				require.Error(t, runErr)
+				require.Contains(t, runErr.Error(), tt.wantErr)
+				require.Contains(t, runErr.Error(), server.URL)
+				return
+			}
+			require.NoError(t, runErr)
+		})
+	}
+}
+
+func TestResolveOIDCRealm(t *testing.T) {
+	tests := []struct {
+		name            string
+		args            []string
+		configRealm     string
+		wantRealm       string
+		wantFromDefault bool
+	}{
+		{
+			name:            "falls back to the flag default",
+			wantRealm:       "nico-dev",
+			wantFromDefault: true,
+		},
+		{
+			name:        "configured realm beats the flag default",
+			configRealm: "nico",
+			wantRealm:   "nico",
+		},
+		{
+			name:        "explicit flag beats the configured realm",
+			args:        []string{"--keycloak-realm", "nico-prod"},
+			configRealm: "nico",
+			wantRealm:   "nico-prod",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			withArgs(t, append([]string{"nicocli"}, tt.args...)...)
+			c := newRealmFlagContext(t, tt.args)
+			cfg := &ConfigFile{Auth: ConfigAuth{OIDC: &ConfigOIDC{Realm: tt.configRealm}}}
+
+			realm, fromDefault := resolveOIDCRealm(c, cfg)
+			require.Equal(t, tt.wantRealm, realm)
+			require.Equal(t, tt.wantFromDefault, fromDefault)
+		})
+	}
+}
+
+func TestResolveOIDCClientID(t *testing.T) {
+	tests := []struct {
+		name            string
+		args            []string
+		configClientID  string
+		wantClientID    string
+		wantFromDefault bool
+	}{
+		{
+			name:            "falls back to the flag default",
+			wantClientID:    "nico-api",
+			wantFromDefault: true,
+		},
+		{
+			name:           "configured client beats the flag default",
+			configClientID: "nico-rest",
+			wantClientID:   "nico-rest",
+		},
+		{
+			name:           "explicit flag beats the configured client",
+			args:           []string{"--client-id", "override-id"},
+			configClientID: "nico-rest",
+			wantClientID:   "override-id",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			withArgs(t, append([]string{"nicocli"}, tt.args...)...)
+			c := newClientIDFlagContext(t, tt.args)
+			cfg := &ConfigFile{Auth: ConfigAuth{OIDC: &ConfigOIDC{ClientID: tt.configClientID}}}
+
+			clientID, fromDefault := resolveOIDCClientID(c, cfg)
+			require.Equal(t, tt.wantClientID, clientID)
+			require.Equal(t, tt.wantFromDefault, fromDefault)
+		})
+	}
+}
+
+func TestLoginFailureHint(t *testing.T) {
+	tests := []struct {
+		name        string
+		defaulted   []string
+		wantContain []string
+		wantAbsent  string
+	}{
+		{
+			name:        "reports only the endpoint when nothing was defaulted",
+			wantContain: []string{"token endpoint: https://kc.example/token"},
+			wantAbsent:  "built-in default",
+		},
+		{
+			name:        "names a single default",
+			defaulted:   []string{"--keycloak-realm=nico-dev"},
+			wantContain: []string{"built-in default --keycloak-realm=nico-dev", "auth.oidc"},
+		},
+		{
+			name:        "joins multiple defaults",
+			defaulted:   []string{"--keycloak-realm=nico-dev", "--client-id=nico-api"},
+			wantContain: []string{"--keycloak-realm=nico-dev and --client-id=nico-api"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hint := loginFailureHint("https://kc.example/token", tt.defaulted)
+			for _, want := range tt.wantContain {
+				require.Contains(t, hint, want)
+			}
+			if tt.wantAbsent != "" {
+				require.NotContains(t, hint, tt.wantAbsent)
+			}
 		})
 	}
 }
 
+// newRealmFlagContext builds a cli.Context carrying only the keycloak-realm flag, with
+// the same default the login command declares.
+func newRealmFlagContext(t *testing.T, args []string) *cli.Context {
+	t.Helper()
+	set := flag.NewFlagSet("test", flag.ContinueOnError)
+	set.String("keycloak-realm", "nico-dev", "")
+	require.NoError(t, set.Parse(args))
+	return cli.NewContext(nil, set, nil)
+}
+
+// newClientIDFlagContext builds a cli.Context carrying only the client-id flag, with the
+// same default the login command declares.
+func newClientIDFlagContext(t *testing.T, args []string) *cli.Context {
+	t.Helper()
+	set := flag.NewFlagSet("test", flag.ContinueOnError)
+	set.String("client-id", "nico-api", "")
+	require.NoError(t, set.Parse(args))
+	return cli.NewContext(nil, set, nil)
+}
+
 func TestExtractNGCToken(t *testing.T) {
 	tests := []struct {
 		name string