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

Add Client option for access_token_class #516

Merged

Conversation

jonspalmer
Copy link
Contributor

@jonspalmer jonspalmer commented Jul 13, 2020

The Problem

There is no way to use OAuth2::Client strategies such as auth_code AND supply a custom access_token_class.

OAuth2::Client.get_token has an optional argument for the access_token_class to instantiate.
However, callers of it have to know to use it and none of the strategies do so. This means the anyone needing a custom access_token_class, #243 or #511, need to jump through hoops to get the behavior they want. For example subclassing OAuth2::Client just to change the default class

What I did

Added a new option, access_token_class, to OAuth2::Client which is used as the default access_token_class for all calls to get_token.

This is a safe, non-breaking change.

@coveralls
Copy link

coveralls commented Jul 13, 2020

Pull Request Test Coverage Report for Build 1040

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 1038: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

@pboling
Copy link
Member

pboling commented Jul 22, 2020

@jonspalmer Would it be too complex to bother trying this on the 1-4-stable branch?

@jonspalmer
Copy link
Contributor Author

@jonspalmer Would it be too complex to bother trying this on the 1-4-stable branch?

@pboling #518 provides a better solution to this problem and is already in 1-4-stable. That change is going to merge conflict with this change in master and IMO should replace this change entirely. The point being that the proc approach in #518 allows you to return any flavor of AccessToken you desire.

@pboling
Copy link
Member

pboling commented May 9, 2021

@jonspalmer Thanks for the heads up, again, and sorry for the long delay in addressing it. v2 is moving ahead at a very slow pace because my personal use case for this gem is vanishingly thin*. I would need far more community support to move it forward more quickly.

I'm going to work on parity between 1-4-stable and v2 (master, soon to be main) now.

* none of the work done, or bugs fixed, since I took over maintenance have affected my use case. I'm donating time to the community because I want the community to survive, and I know this gem matters a great deal to many important projects.

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