From f8af13025a18f85aa1ef37ed62de343b2b5d2d8f Mon Sep 17 00:00:00 2001 From: Jose Antonio Insua Date: Fri, 27 Feb 2026 15:59:37 +0100 Subject: [PATCH 1/5] Use IMS base URL for DCR registration instead of separate --url flag Remove the RegisterURL config field and --url flag from the dcr register command. The registration endpoint is now derived from the IMS base URL (persistent --url/-U flag) by appending /ims/register. --- cmd/dcr.go | 1 - ims/config.go | 1 - ims/config_test.go | 8 ++++---- ims/register.go | 8 +++++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/dcr.go b/cmd/dcr.go index c794984..7ca9116 100644 --- a/cmd/dcr.go +++ b/cmd/dcr.go @@ -39,7 +39,6 @@ func registerCmd(imsConfig *ims.Config) *cobra.Command { }, } - cmd.Flags().StringVarP(&imsConfig.RegisterURL, "url", "u", "", "Registration endpoint URL.") cmd.Flags().StringVarP(&imsConfig.ClientName, "clientName", "n", "", "Client application name.") cmd.Flags().StringSliceVarP(&imsConfig.RedirectURIs, "redirectURIs", "r", []string{}, "Redirect URIs (comma-separated or multiple flags).") diff --git a/ims/config.go b/ims/config.go index 103e4e2..5c9dd82 100644 --- a/ims/config.go +++ b/ims/config.go @@ -46,7 +46,6 @@ type Config struct { Guid string AuthSrc string DecodeFulfillableData bool - RegisterURL string ClientName string RedirectURIs []string } diff --git a/ims/config_test.go b/ims/config_test.go index 905d6df..ced8041 100644 --- a/ims/config_test.go +++ b/ims/config_test.go @@ -295,10 +295,10 @@ func TestValidateRegisterConfig(t *testing.T) { config Config wantErr string }{ - {name: "valid", config: Config{RegisterURL: "https://example.com/register", ClientName: "app", RedirectURIs: []string{"https://example.com/cb"}}, wantErr: ""}, - {name: "missing URL", config: Config{ClientName: "app", RedirectURIs: []string{"https://example.com/cb"}}, wantErr: "missing registration endpoint URL"}, - {name: "missing client name", config: Config{RegisterURL: "https://example.com/register", RedirectURIs: []string{"https://example.com/cb"}}, wantErr: "missing client name"}, - {name: "missing redirect URIs", config: Config{RegisterURL: "https://example.com/register", ClientName: "app"}, wantErr: "missing redirect URIs"}, + {name: "valid", config: Config{URL: "https://ims.example.com", ClientName: "app", RedirectURIs: []string{"https://example.com/cb"}}, wantErr: ""}, + {name: "missing URL", config: Config{ClientName: "app", RedirectURIs: []string{"https://example.com/cb"}}, wantErr: "missing IMS base URL"}, + {name: "missing client name", config: Config{URL: "https://ims.example.com", RedirectURIs: []string{"https://example.com/cb"}}, wantErr: "missing client name"}, + {name: "missing redirect URIs", config: Config{URL: "https://ims.example.com", ClientName: "app"}, wantErr: "missing redirect URIs"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/ims/register.go b/ims/register.go index 88e94d8..7f70e8c 100644 --- a/ims/register.go +++ b/ims/register.go @@ -15,8 +15,8 @@ type RegisterResponse struct { func (i Config) validateRegisterConfig() error { switch { - case i.RegisterURL == "": - return fmt.Errorf("missing registration endpoint URL parameter") + case i.URL == "": + return fmt.Errorf("missing IMS base URL parameter") case i.ClientName == "": return fmt.Errorf("missing client name parameter") case len(i.RedirectURIs) == 0: @@ -47,7 +47,9 @@ func (i Config) Register() (RegisterResponse, error) { "redirect_uris": %s }`, i.ClientName, redirectURIsJSON)) - req, err := http.NewRequest("POST", i.RegisterURL, payload) + endpoint := strings.TrimRight(i.URL, "/") + "/ims/register" + + req, err := http.NewRequest("POST", endpoint, payload) if err != nil { return RegisterResponse{}, fmt.Errorf("error creating request: %v", err) } From d46aae8bb62ec346099ee512db830efb5c3a8b98 Mon Sep 17 00:00:00 2001 From: Jose Antonio Insua Date: Fri, 27 Feb 2026 16:07:33 +0100 Subject: [PATCH 2/5] Standardize flag shorthands across all commands - -p always means --clientSecret (was -s in authz jwt and invalidate serviceToken) - -s always means --scopes (no longer overloaded) - -a always means --*ApiVersion (no longer overloaded) - --account gets -A, --authorizationCode gets -x, --cascading gets -C, --authSrc gets -A --- cmd/admin/organizations.go | 2 +- cmd/admin/profile.go | 2 +- cmd/authz/jwt.go | 4 ++-- cmd/authz/service.go | 2 +- cmd/invalidate/invalidate.go | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/admin/organizations.go b/cmd/admin/organizations.go index 5318d35..8e2759b 100644 --- a/cmd/admin/organizations.go +++ b/cmd/admin/organizations.go @@ -38,7 +38,7 @@ func OrganizationsCmd(imsConfig *ims.Config) *cobra.Command { }, } cmd.Flags().StringVarP(&imsConfig.Guid, "guid", "g", "", "User ID.") - cmd.Flags().StringVarP(&imsConfig.AuthSrc, "authSrc", "s", "", "Authorization source.") + cmd.Flags().StringVarP(&imsConfig.AuthSrc, "authSrc", "A", "", "Authorization source.") cmd.Flags().StringVarP(&imsConfig.ClientID, "clientID", "c", "", "IMS client ID.") cmd.Flags().StringVarP(&imsConfig.ServiceToken, "serviceToken", "t", "", "Service token.") cmd.Flags().StringVarP(&imsConfig.OrgsAPIVersion, "orgsApiVersion", "a", "v5", "Admin organizations API version.") diff --git a/cmd/admin/profile.go b/cmd/admin/profile.go index bcce154..9904ad8 100644 --- a/cmd/admin/profile.go +++ b/cmd/admin/profile.go @@ -36,7 +36,7 @@ func ProfileCmd(imsConfig *ims.Config) *cobra.Command { }, } cmd.Flags().StringVarP(&imsConfig.Guid, "guid", "g", "", "User ID.") - cmd.Flags().StringVarP(&imsConfig.AuthSrc, "authSrc", "s", "", "Authorization source.") + cmd.Flags().StringVarP(&imsConfig.AuthSrc, "authSrc", "A", "", "Authorization source.") cmd.Flags().StringVarP(&imsConfig.ClientID, "clientID", "c", "", "IMS client ID.") cmd.Flags().StringVarP(&imsConfig.ServiceToken, "serviceToken", "t", "", "Service token.") cmd.Flags().StringVarP(&imsConfig.ProfileAPIVersion, "profileApiVersion", "a", "v1", "Admin profile API version.") diff --git a/cmd/authz/jwt.go b/cmd/authz/jwt.go index 9ab54d2..90898c7 100644 --- a/cmd/authz/jwt.go +++ b/cmd/authz/jwt.go @@ -36,9 +36,9 @@ func JWTCmd(imsConfig *ims.Config) *cobra.Command { } cmd.Flags().StringVarP(&imsConfig.ClientID, "clientID", "c", "", "IMS Client ID.") - cmd.Flags().StringVarP(&imsConfig.ClientSecret, "clientSecret", "s", "", "IMS Client secret.") + cmd.Flags().StringVarP(&imsConfig.ClientSecret, "clientSecret", "p", "", "IMS Client secret.") cmd.Flags().StringVarP(&imsConfig.Organization, "organization", "o", "", "IMS Organization.") - cmd.Flags().StringVarP(&imsConfig.Account, "account", "a", "", "Technical Account ID.") + cmd.Flags().StringVarP(&imsConfig.Account, "account", "A", "", "Technical Account ID.") cmd.Flags().StringVarP(&imsConfig.PrivateKeyPath, "privateKey", "k", "", "Private Key file.") cmd.Flags().StringSliceVarP(&imsConfig.Metascopes, "metascopes", "m", []string{}, "Metascopes to request.") diff --git a/cmd/authz/service.go b/cmd/authz/service.go index 391e58c..c47e272 100644 --- a/cmd/authz/service.go +++ b/cmd/authz/service.go @@ -37,7 +37,7 @@ func ServiceCmd(imsConfig *ims.Config) *cobra.Command { cmd.Flags().StringVarP(&imsConfig.ClientID, "clientID", "c", "", "IMS client ID.") cmd.Flags().StringVarP(&imsConfig.ClientSecret, "clientSecret", "p", "", "IMS client secret.") - cmd.Flags().StringVarP(&imsConfig.AuthorizationCode, "authorizationCode", "a", "", "Permanent authorization code.") + cmd.Flags().StringVarP(&imsConfig.AuthorizationCode, "authorizationCode", "x", "", "Permanent authorization code.") return cmd } diff --git a/cmd/invalidate/invalidate.go b/cmd/invalidate/invalidate.go index 714b166..4a5f82a 100644 --- a/cmd/invalidate/invalidate.go +++ b/cmd/invalidate/invalidate.go @@ -68,7 +68,7 @@ func RefreshTokenCmd(imsConfig *ims.Config) *cobra.Command { flagName: "refreshToken", field: &imsConfig.RefreshToken, successMsg: "Refresh token successfully invalidated.", extraFlags: func(cmd *cobra.Command, c *ims.Config) { - cmd.Flags().BoolVarP(&c.Cascading, "cascading", "a", false, + cmd.Flags().BoolVarP(&c.Cascading, "cascading", "C", false, "Also invalidate all tokens obtained with the refresh token.") }, }) @@ -80,7 +80,7 @@ func DeviceTokenCmd(imsConfig *ims.Config) *cobra.Command { flagName: "deviceToken", field: &imsConfig.DeviceToken, successMsg: "Token invalidated successfully.", extraFlags: func(cmd *cobra.Command, c *ims.Config) { - cmd.Flags().BoolVarP(&c.Cascading, "cascading", "a", false, + cmd.Flags().BoolVarP(&c.Cascading, "cascading", "C", false, "Also invalidate all tokens obtained with the device token.") }, }) @@ -92,7 +92,7 @@ func ServiceTokenCmd(imsConfig *ims.Config) *cobra.Command { flagName: "serviceToken", field: &imsConfig.ServiceToken, successMsg: "Service token successfully invalidated.", extraFlags: func(cmd *cobra.Command, c *ims.Config) { - cmd.Flags().StringVarP(&c.ClientSecret, "clientSecret", "s", "", "IMS Client Secret.") + cmd.Flags().StringVarP(&c.ClientSecret, "clientSecret", "p", "", "IMS Client Secret.") }, }) } From c0d1fca2e94c4e912183fad2c5f2976bdbcc5bfc Mon Sep 17 00:00:00 2001 From: Jose Antonio Insua Date: Fri, 27 Feb 2026 16:24:39 +0100 Subject: [PATCH 3/5] Split AuthorizeUser into AuthorizeUser and AuthorizeUserPKCE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove PKCE field from Config — it was not user configuration but a command-level decision. The pkce command now calls AuthorizeUserPKCE() directly instead of mutating the shared Config pointer. --- cmd/authz/pkce.go | 3 +-- ims/authz_user.go | 14 +++++++++++--- ims/config.go | 1 - 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cmd/authz/pkce.go b/cmd/authz/pkce.go index 463e286..36d4e76 100644 --- a/cmd/authz/pkce.go +++ b/cmd/authz/pkce.go @@ -27,8 +27,7 @@ func UserPkceCmd(imsConfig *ims.Config) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true - imsConfig.PKCE = true - resp, err := imsConfig.AuthorizeUser() + resp, err := imsConfig.AuthorizeUserPKCE() if err != nil { return fmt.Errorf("error in user authorization: %w", err) } diff --git a/ims/authz_user.go b/ims/authz_user.go index b02cba2..83e252e 100644 --- a/ims/authz_user.go +++ b/ims/authz_user.go @@ -55,9 +55,17 @@ func (i Config) validateAuthorizeUserConfig() error { return nil } -// AuthorizeUser uses the standard Oauth2 authorization code grant flow. The Oauth2 configuration is -// taken from the Config struct. +// AuthorizeUser uses the standard OAuth2 authorization code grant flow. func (i Config) AuthorizeUser() (string, error) { + return i.authorizeUser(false) +} + +// AuthorizeUserPKCE uses the OAuth2 authorization code grant flow with PKCE. +func (i Config) AuthorizeUserPKCE() (string, error) { + return i.authorizeUser(true) +} + +func (i Config) authorizeUser(pkce bool) (string, error) { // Perform parameter validation err := i.validateAuthorizeUserConfig() if err != nil { @@ -80,7 +88,7 @@ func (i Config) AuthorizeUser() (string, error) { ClientID: i.ClientID, ClientSecret: i.ClientSecret, Scope: i.Scopes, - UsePKCE: i.PKCE, + UsePKCE: pkce, RedirectURI: fmt.Sprintf("http://localhost:%d", port), OnError: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, ` diff --git a/ims/config.go b/ims/config.go index 5c9dd82..371f468 100644 --- a/ims/config.go +++ b/ims/config.go @@ -41,7 +41,6 @@ type Config struct { Cascading bool Token string Port int - PKCE bool FullOutput bool Guid string AuthSrc string From d6badce3faaaf87c26b3f5d798c41ddc1a6d3528 Mon Sep 17 00:00:00 2001 From: Jose Antonio Insua Date: Fri, 27 Feb 2026 17:13:10 +0100 Subject: [PATCH 4/5] Miscellaneous hardening: zero private key, port validation, struct JSON, URL slash - Zero private key bytes after use in JWT exchange to prevent key material lingering in memory until GC (6.1.7) - Add port validation to validateAuthorizeUserConfig, remove defaultPort fallback so the ims package validates its own parameters (N10) - Replace map[string]interface{} with anonymous struct in refresh command for deterministic JSON field order and better performance (N11) - Trim trailing slash from IMS URL before building JWT claim keys to prevent double-slash in metascope URIs (6.2.10) - Standardize orgs API version error message to match profile style (6.2.12) - Document PersistentPreRunE override risk in README.md (N7) Co-Authored-By: Claude Opus 4.6 --- README.md | 6 ++++++ cmd/refresh.go | 8 ++++---- ims/authz_user.go | 18 ++++++------------ ims/config_test.go | 2 ++ ims/jwt_exchange.go | 9 ++++++++- ims/organizations.go | 2 +- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 6b9f030..5652c8a 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,12 @@ imscli authorize user help The complete documentation of the project is available in the [DOCUMENTATION.md](DOCUMENTATION.md) file. +## Development Notes + +### PersistentPreRunE and subcommands + +The root command defines a `PersistentPreRunE` that loads configuration from flags, environment variables, and config files (see `cmd/root.go`). In cobra, if a subcommand defines its own `PersistentPreRunE`, it **overrides** the parent's — the root's `PersistentPreRunE` will not run for that subcommand or its children. If you need to add a `PersistentPreRunE` to a subcommand, you must explicitly call the parent's first. + ## Contributing Contributions are welcomed! Read the [Contributing Guide](CONTRIBUTING.md) for more information. diff --git a/cmd/refresh.go b/cmd/refresh.go index 648b59b..877dfb7 100644 --- a/cmd/refresh.go +++ b/cmd/refresh.go @@ -33,10 +33,10 @@ func refreshCmd(imsConfig *ims.Config) *cobra.Command { return fmt.Errorf("error during the token refresh: %w", err) } if imsConfig.FullOutput { - data := map[string]interface{}{ - "access_token": resp.AccessToken, - "refresh_token": resp.RefreshToken, - } + data := struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + }{resp.AccessToken, resp.RefreshToken} jsonData, err := json.MarshalIndent(data, "", " ") if err != nil { return fmt.Errorf("error marshalling full JSON response: %w", err) diff --git a/ims/authz_user.go b/ims/authz_user.go index 83e252e..07685e2 100644 --- a/ims/authz_user.go +++ b/ims/authz_user.go @@ -24,8 +24,6 @@ import ( "github.com/pkg/browser" ) -const defaultPort = 8888 - // Validate that: // - the ims.Config struct has the necessary parameters for AuthorizeUser // - the provided environment exists @@ -42,6 +40,8 @@ func (i Config) validateAuthorizeUserConfig() error { return fmt.Errorf("missing client id parameter") case i.Organization == "": return fmt.Errorf("missing organization parameter") + case i.Port <= 0: + return fmt.Errorf("missing or invalid port parameter") case i.ClientSecret == "": if i.PublicClient { log.Println("all needed parameters verified not empty") @@ -72,12 +72,6 @@ func (i Config) authorizeUser(pkce bool) (string, error) { return "", fmt.Errorf("invalid parameters for login user: %w", err) } - // Use default port if not specified - port := i.Port - if port == 0 { - port = defaultPort - } - c, err := i.newIMSClient() if err != nil { return "", fmt.Errorf("error creating the IMS client: %w", err) @@ -89,7 +83,7 @@ func (i Config) authorizeUser(pkce bool) (string, error) { ClientSecret: i.ClientSecret, Scope: i.Scopes, UsePKCE: pkce, - RedirectURI: fmt.Sprintf("http://localhost:%d", port), + RedirectURI: fmt.Sprintf("http://localhost:%d", i.Port), OnError: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, `

An error occurred

@@ -107,15 +101,15 @@ func (i Config) authorizeUser(pkce bool) (string, error) { return "", fmt.Errorf("create authorization server: %w", err) } - listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port)) + listener, err := net.Listen("tcp", fmt.Sprintf(":%d", i.Port)) if err != nil { - return "", fmt.Errorf("unable to listen at port %d", port) + return "", fmt.Errorf("unable to listen at port %d", i.Port) } defer listener.Close() log.Println("Local server successfully launched and contacted.") - localUrl := fmt.Sprintf("http://localhost:%d/", port) + localUrl := fmt.Sprintf("http://localhost:%d/", i.Port) // Suppress "Opening in existing browser session." messages from chromium-based // browsers. The CLI token output goes to stdout, so stray browser messages diff --git a/ims/config_test.go b/ims/config_test.go index ced8041..c628844 100644 --- a/ims/config_test.go +++ b/ims/config_test.go @@ -224,6 +224,7 @@ func TestValidateAuthorizeUserConfig(t *testing.T) { ClientSecret: "s", Organization: "org", Scopes: []string{"openid"}, + Port: 8888, } tests := []struct { name string @@ -239,6 +240,7 @@ func TestValidateAuthorizeUserConfig(t *testing.T) { {name: "missing clientID", config: withField(validConfig, func(c *Config) { c.ClientID = "" }), wantErr: "missing client id"}, {name: "missing organization", config: withField(validConfig, func(c *Config) { c.Organization = "" }), wantErr: "missing organization"}, {name: "missing secret non-public", config: withField(validConfig, func(c *Config) { c.ClientSecret = "" }), wantErr: "missing client secret"}, + {name: "missing port", config: withField(validConfig, func(c *Config) { c.Port = 0 }), wantErr: "missing or invalid port"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/ims/jwt_exchange.go b/ims/jwt_exchange.go index a30277b..a1d7ba2 100644 --- a/ims/jwt_exchange.go +++ b/ims/jwt_exchange.go @@ -13,6 +13,7 @@ package ims import ( "fmt" "os" + "strings" "time" "github.com/adobe/ims-go/ims" @@ -29,14 +30,20 @@ func (i Config) AuthorizeJWTExchange() (TokenInfo, error) { if err != nil { return TokenInfo{}, fmt.Errorf("error read private key file: %s, %w", i.PrivateKeyPath, err) } + defer func() { + for i := range key { + key[i] = 0 + } + }() // Metascopes are passed as generic claims with the format map[string]interface{} // where the strings are in the form: baseIMSUrl/s/metascope // and the interface{} is 'true' + baseURL := strings.TrimRight(i.URL, "/") claims := make(map[string]interface{}) for _, metascope := range i.Metascopes { - claims[fmt.Sprintf("%s/s/%s", i.URL, metascope)] = true + claims[fmt.Sprintf("%s/s/%s", baseURL, metascope)] = true } r, err := c.ExchangeJWT(&ims.ExchangeJWTRequest{ diff --git a/ims/organizations.go b/ims/organizations.go index 5853c9b..c9fadc4 100644 --- a/ims/organizations.go +++ b/ims/organizations.go @@ -22,7 +22,7 @@ func (i Config) validateGetOrganizationsConfig() error { switch i.OrgsAPIVersion { case "v1", "v2", "v3", "v4", "v5", "v6": default: - return fmt.Errorf("invalid API version parameter, use something like v5") + return fmt.Errorf("invalid API version parameter, latest version is v6") } switch { From 8a68d34ec3cd85ea7950c8bf27b8cff0c51d6e5f Mon Sep 17 00:00:00 2001 From: Jose Antonio Insua Date: Fri, 27 Feb 2026 17:26:42 +0100 Subject: [PATCH 5/5] Remove dead TokenInfo.Expires field and broken calculations The Expires field was set in jwt_exchange.go, exchange.go, and refresh.go but never read by any caller. The calculations were also wrong: duration * duration produces nanoseconds-squared (overflow), and refresh.go used time.Second while the others used time.Millisecond. Remove the field entirely rather than fixing dead code. Co-Authored-By: Claude Opus 4.6 --- ims/config.go | 1 - ims/exchange.go | 2 -- ims/jwt_exchange.go | 1 - ims/refresh.go | 3 +-- 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/ims/config.go b/ims/config.go index 371f468..8500eca 100644 --- a/ims/config.go +++ b/ims/config.go @@ -52,7 +52,6 @@ type Config struct { // Access token information type TokenInfo struct { AccessToken string - Expires int //(response.ExpiresIn * time.Millisecond), Valid bool Info string } diff --git a/ims/exchange.go b/ims/exchange.go index 6bd1d43..c1405d4 100644 --- a/ims/exchange.go +++ b/ims/exchange.go @@ -12,7 +12,6 @@ package ims import ( "fmt" - "time" "github.com/adobe/ims-go/ims" ) @@ -62,6 +61,5 @@ func (i Config) ClusterExchange() (TokenInfo, error) { return TokenInfo{ AccessToken: r.AccessToken, - Expires: int(r.ExpiresIn * time.Millisecond), }, nil } diff --git a/ims/jwt_exchange.go b/ims/jwt_exchange.go index a1d7ba2..42a91f4 100644 --- a/ims/jwt_exchange.go +++ b/ims/jwt_exchange.go @@ -61,6 +61,5 @@ func (i Config) AuthorizeJWTExchange() (TokenInfo, error) { return TokenInfo{ AccessToken: r.AccessToken, - Expires: int(r.ExpiresIn * time.Millisecond), }, nil } diff --git a/ims/refresh.go b/ims/refresh.go index 185a258..82ce08e 100644 --- a/ims/refresh.go +++ b/ims/refresh.go @@ -12,8 +12,8 @@ package ims import ( "fmt" + "github.com/adobe/ims-go/ims" - "time" ) func (i Config) validateRefreshConfig() error { @@ -56,7 +56,6 @@ func (i Config) Refresh() (RefreshInfo, error) { return RefreshInfo{ TokenInfo: TokenInfo{ AccessToken: r.AccessToken, - Expires: int(r.ExpiresIn * time.Second), }, RefreshToken: r.RefreshToken, }, nil