Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adds access token authentication to client #166

Merged
merged 3 commits into from
Aug 22, 2023

Conversation

geofflamrock
Copy link
Contributor

@geofflamrock geofflamrock commented Aug 21, 2023

This PR adds support for authenticating to the Octopus API using an access token as a bearer token in the Authorization header, as part of supporting OIDC for connecting to the API.

The apiKey property in the ClientConfiguration type has been made optional, and backward compatibility has been preserved in the handling of headers, such that if none are provided then X-Octopus-ApiKey will be passed with an empty string.

Note: The intent is to treat this as a minor version bump as the changes are backward compatible.

[sc-54886]

const configuration: ClientConfiguration = {
instanceURL: "https://my.octopus.app",
userAgentApp: "createRequestHeader-tests",
accessToken: "access-token",

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "access-token" is used as [authorization header](1).

expect(headers).toEqual({
"Accept-Encoding": "gzip,deflate,compress",
Authorization: "Bearer access-token",

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "Bearer access-token" is used as [authorization header](1).
@@ -12,12 +13,10 @@ export class AxiosAdapter<TResource> implements Adapter<TResource> {
url: options.url,
maxContentLength: Infinity,
maxBodyLength: Infinity,
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions

Choose a reason for hiding this comment

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

Why did these light up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah not sure why they wouldn't have turned up before, as soon as I opened the file it lit up. We have these in a bunch of places across our various typescript codebases where we need them, so I think the disable is fine in an of itself.

Copy link

@dylanlerch dylanlerch left a comment

Choose a reason for hiding this comment

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

Looks good. Just one half question about why we've had to disable some warnings, but not a blocking thing. Assuming the disable has reasons.

@geofflamrock geofflamrock changed the title feat: Adds access token to client feat: Adds access token authentication to client Aug 22, 2023
@geofflamrock geofflamrock merged commit 7ba6d05 into main Aug 22, 2023
@geofflamrock geofflamrock deleted the geoffl/access-token-auth branch August 22, 2023 02:58
@shortcut-integration
Copy link

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.

2 participants