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(IAM Authenticator): add support for optional 'scope' property #109

Merged
merged 1 commit into from
Sep 18, 2020

Conversation

jorge-ibm
Copy link
Contributor

The IAM /identity/token (get token) operation supports an optional "scope" form parameter which can be used to obtain an IAM access token having a limited set of scopes in this it can be used.

This commit adds support for the "Scope" field within the IamTokenManager & IamAuthenticator class. If set, the value of this field is sent as the "scope" form param in the "get token" request body when obtaining an access token.

ref: https://github.ibm.com/arf/planning-sdk-squad/issues/2167

Checklist
  • npm test passes (tip: npm run lint-fix can correct most style issues)
  • tests are included
  • documentation is changed or added

Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

Awesome work. There is just one small typo that must be fixed before merging but I am approving to avoid the need for a re-review. I left some other suggestions that you can take or leave at your own discretion

auth/token-managers/iam-token-manager.ts Outdated Show resolved Hide resolved
auth/token-managers/iam-token-manager.ts Outdated Show resolved Hide resolved
@@ -35,12 +35,14 @@ function onlyOne(a: any, b: any): boolean {
}

const CLIENT_ID_SECRET_WARNING = 'Warning: Client ID and Secret must BOTH be given, or the header will not be included.';
const SCOPE = 'scope';
Copy link
Member

Choose a reason for hiding this comment

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

I don't know that a constant here is really necessary but I also don't have a problem with it. Up to you but I thought I would at least say that. If you find it helpful, I'm fine with it

test/unit/get-authenticator-from-environment.test.js Outdated Show resolved Hide resolved
Copy link
Member

@padamstx padamstx left a comment

Choose a reason for hiding this comment

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

LGTM

@jorge-ibm jorge-ibm merged commit 1c258b7 into master Sep 18, 2020
ibm-devx-automation pushed a commit that referenced this pull request Sep 18, 2020
# [2.5.0](v2.4.5...v2.5.0) (2020-09-18)

### Features

* **IAM Authenticator:** add support for optional 'scope' property ([#109](#109)) ([1c258b7](1c258b7))
@ibm-devx-automation
Copy link

🎉 This PR is included in version 2.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@padamstx padamstx deleted the feat/iam-scope branch November 10, 2020 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants