Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

feat: support scopes on compute credentials #642

Merged
JustinBeckwith merged 2 commits into from
Apr 29, 2019
Merged

feat: support scopes on compute credentials #642

JustinBeckwith merged 2 commits into from
Apr 29, 2019

Conversation

@JustinBeckwith
Copy link
Contributor

@JustinBeckwith JustinBeckwith commented Mar 17, 2019 •
edited
Loading

The GCE metadata server is adding support for requesting tokens against user accounts with a predefined set of scopes. This adds experimental support for passing the scopes as querystring parameters to the metadata service in compute credentials.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 17, 2019
@JustinBeckwith JustinBeckwith added do not merge Indicates a pull request not ready for merge, due to either quality or timing. needs work This is a pull request that needs a little love. labels Mar 17, 2019
@codecov
Copy link

codecov bot commented Mar 17, 2019

Codecov Report

Merging #642 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #642      +/-   ##
==========================================
+ Coverage   88.25%   88.31%   +0.05%     
==========================================
  Files          18       18              
  Lines         783      787       +4     
  Branches       86       87       +1     
==========================================
+ Hits          691      695       +4     
  Misses         80       80              
  Partials       12       12
Impacted Files Coverage Δ
src/auth/googleauth.ts 91% <100%> (+0.09%) ⬆️
src/auth/computeclient.ts 94.28% <100%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3382506...1f1a473. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 17, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@3016d52). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #642   +/-   ##
=========================================
  Coverage          ?   88.31%           
=========================================
  Files             ?       18           
  Lines             ?      787           
  Branches          ?       87           
=========================================
  Hits              ?      695           
  Misses            ?       80           
  Partials          ?       12
Impacted Files Coverage Δ
src/auth/googleauth.ts 91% <100%> (ø)
src/auth/computeclient.ts 94.28% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3016d52...8fd0056. Read the comment docs.

asciimike
asciimike reviewed Mar 22, 2019
View reviewed changes
@JustinBeckwith JustinBeckwith removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. needs work This is a pull request that needs a little love. labels Apr 29, 2019
@JustinBeckwith
Copy link
Contributor Author

JustinBeckwith commented Apr 29, 2019

@bcoe @alexander-fenster this is ready for a look :)

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 29, 2019
bcoe
bcoe approved these changes Apr 29, 2019
View reviewed changes
Copy link

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

This seems good to me.

If I understand the code properly, does this mean that we need to set the same options when generating a refresh token as when we initially request an access token?

src/auth/computeclient.ts
property: tokenPath,
params: {
scopes: this.scopes
// TODO: clean up before submit, fix upstream type bug
Copy link

@bcoe bcoe Apr 29, 2019

Choose a reason for hiding this comment

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

is this still a TODO prior to landing?

alexander-fenster
alexander-fenster reviewed Apr 29, 2019
View reviewed changes
@JustinBeckwith JustinBeckwith merged commit 1811b7f into master Apr 29, 2019
@alexander-fenster alexander-fenster deleted the compute-scopes branch May 8, 2019 19:14
@busunkim96 busunkim96 mentioned this pull request Aug 11, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@asciimike asciimike asciimike left review comments

@alexander-fenster alexander-fenster alexander-fenster left review comments

@bcoe bcoe bcoe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@JustinBeckwith @bcoe @asciimike @alexander-fenster @googlebot @yoshi-automation