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.

Sign up for GitHub

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

Jump to bottom

Add simple credentials class. #122

Merged
merged 4 commits into from
Oct 12, 2017
Merged

Add simple credentials class. #122

merged 4 commits into from
Oct 12, 2017

Conversation

landrito
Copy link
Contributor

@landrito landrito commented Oct 2, 2017 •
edited
Loading

What

This introduces a class that allows for the following ways of instantiating an OAuth2 Signet.

  1. Instantiating with a keyfile.
  2. Instantiating with a hash filled with keyfile information.
  3. Instantiating with user defined Environment Variables.

This class is a direct port of the credentials class in the google-cloud-core gem.

Why

For the case of using multiple APIs within the same project, we needed a way for the user to default locations for their keyfiles on a per api basis. By extending this class, we can define api specific locations to store default credentials.

For context, we were going to have this class live in our general apis utilities library google-gax but it would introduce a dependency on grpc for http client libraries that would use this class.

cc @geigerj

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 2, 2017
@coveralls
Copy link

coveralls commented Oct 2, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-1.8%) to 95.694% when pulling 7520c28 on landrito:api-creds into 7ee0810 on google:master.

@geigerj
Copy link
Contributor

geigerj commented Oct 2, 2017

@landrito I think you are seeing failures because this library supports Ruby 1.9, which doesn't have keyword args.

@landrito
Copy link
Contributor Author

landrito commented Oct 2, 2017

Yep let me make a fix.

@geigerj
Copy link
Contributor

geigerj commented Oct 2, 2017

@dazuma Would either you or Heng be able do the review for this PR?

@dazuma
Copy link
Member

dazuma commented Oct 3, 2017

I can do the review

@dazuma dazuma self-requested a review October 3, 2017 23:37
@landrito
Copy link
Contributor Author

landrito commented Oct 3, 2017

Thanks @dazuma! I'll add some context to this PR and get it working for 1.9.3 sometime tonight.

@dazuma
Copy link
Member

dazuma commented Oct 5, 2017

@landrito Is this ready for review?

@landrito
Copy link
Contributor Author

landrito commented Oct 5, 2017

Not yet. Will ping you when ready. Sometime today.

@coveralls
Copy link

coveralls commented Oct 5, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.9%) to 96.552% when pulling dd36265 on landrito:api-creds into 7ee0810 on google:master.

@coveralls
Copy link

coveralls commented Oct 5, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.9%) to 96.552% when pulling dd36265 on landrito:api-creds into 7ee0810 on google:master.

@landrito
Copy link
Contributor Author

landrito commented Oct 5, 2017

Ready for a look!

dazuma
dazuma reviewed Oct 6, 2017
View reviewed changes
lib/googleauth/credentials.rb Outdated
new client
end

def self.env(v)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

lib/googleauth/default_credentials.rb Outdated
end
end

# Obtains the default credentials implementation to use in this

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

lib/googleauth/default_credentials.rb Outdated
require 'stringio'

require 'googleauth/credentials_loader'
require 'googleauth/client_id'

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

lib/googleauth/credentials.rb
TOKEN_CREDENTIAL_URI = 'https://accounts.google.com/o/oauth2/token'.freeze
AUDIENCE = 'https://accounts.google.com/o/oauth2/token'.freeze
SCOPE = [].freeze
PATH_ENV_VARS = [].freeze

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dazuma
dazuma reviewed Oct 6, 2017
View reviewed changes
lib/googleauth/default_credentials.rb Outdated
# Module Auth provides classes that provide Google-specific authorization
# used to access Google APIs.
module Auth
NOT_FOUND_ERROR = <<END.freeze

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@coveralls
Copy link

coveralls commented Oct 10, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-1.8%) to 95.673% when pulling 1225e37 on landrito:api-creds into 7ee0810 on google:master.

@coveralls
Copy link

coveralls commented Oct 10, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.6%) to 96.896% when pulling f03a14c on landrito:api-creds into 7ee0810 on google:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 96.896% when pulling f03a14c on landrito:api-creds into 7ee0810 on google:master.

@coveralls
Copy link

coveralls commented Oct 10, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-1.8%) to 95.673% when pulling f03a14c on landrito:api-creds into 7ee0810 on google:master.

@landrito landrito mentioned this pull request Oct 10, 2017
Merged
@coveralls
Copy link

coveralls commented Oct 11, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-1.8%) to 95.681% when pulling 94f2910 on landrito:api-creds into 7ee0810 on google:master.

dazuma
dazuma approved these changes Oct 12, 2017
View reviewed changes
Copy link
Member

@dazuma dazuma left a comment

Choose a reason for hiding this comment

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

This LGTM now. Thanks!

I know we want to get a release out to unblock the new codegen, but can we wait a day or so more... I need to push out a new release of signet first, to fix compatibility issues with jwt 2.0.

@dazuma dazuma merged commit c148fc1 into googleapis:master Oct 12, 2017
@geigerj
Copy link
Contributor

geigerj commented Oct 16, 2017

@dazuma Do we have an approx ETA for the fixes that will unblock a new release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dazuma dazuma dazuma approved these changes

@hxiong388 hxiong388 Awaiting requested review from hxiong388

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@landrito @coveralls @geigerj @dazuma @googlebot