Skip to content

Conversation

@andrewpmartinez
Copy link
Member

  • adds test for id config vs cred setting

- adds test for id config vs cred setting
@andrewpmartinez andrewpmartinez requested a review from a team as a code owner October 1, 2025 15:22
idCredentials := edge_apis.NewIdentityCredentialsFromConfig(cfg.ID)
idCredentials.ConfigTypes = cfg.ConfigTypes
cfg.Credentials = idCredentials
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you consider adding a warning in the else here? this might be confusing to people imo. Or return an error ?

Copy link
Member Author

@andrewpmartinez andrewpmartinez Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original implementation required either an ID config or credentials, and then would error.

It was changed to not error so that delayed authentication could occur (i.e. through an access_token from oauth to satisfy an external jwt signer). So not having an id config or credential is not a warning or an error, it is legitimate use case.

Attempting to use an external jwt signer requiers the calling app to be able to make unauthenticated requests to the controller to list the externa jwt signers. The SDK was updated to have those kinds of function calls.

@andrewpmartinez andrewpmartinez merged commit 35de430 into main Oct 1, 2025
14 of 18 checks passed
@andrewpmartinez andrewpmartinez deleted the fix.811.cred.squashing branch October 1, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants