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

feature: social login exchange token #332

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

harunnryd
Copy link

Hi @Nerzal ,

I added a social login exchange token

@harunnryd harunnryd marked this pull request as draft January 3, 2022 09:59
@harunnryd harunnryd marked this pull request as ready for review January 3, 2022 10:00
@harunnryd harunnryd marked this pull request as draft January 3, 2022 10:21
@harunnryd harunnryd closed this Jan 3, 2022
@harunnryd harunnryd reopened this Jan 3, 2022
@harunnryd harunnryd marked this pull request as ready for review January 3, 2022 10:29
@codecov
Copy link

codecov bot commented Jan 3, 2022

Codecov Report

Merging #332 (254e85e) into main (0246ee9) will decrease coverage by 1.05%.
The diff coverage is 0.00%.

❗ Current head 254e85e differs from pull request most recent head b99de2d. Consider uploading reports for the commit b99de2d to get more accurate results

@@            Coverage Diff             @@
##             main     #332      +/-   ##
==========================================
- Coverage   77.26%   76.21%   -1.05%     
==========================================
  Files           4        4              
  Lines        2261     1972     -289     
==========================================
- Hits         1747     1503     -244     
+ Misses        345      329      -16     
+ Partials      169      140      -29     
Files Coverage Δ
models.go 92.80% <ø> (+2.42%) ⬆️
client.go 74.38% <0.00%> (-1.45%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

client.go Outdated Show resolved Hide resolved
@harunnryd harunnryd requested a review from Nerzal January 13, 2022 15:04
Nerzal
Nerzal previously approved these changes Jan 14, 2022
Copy link
Owner

@Nerzal Nerzal left a comment

Choose a reason for hiding this comment

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

LGTM :)
A new tag will be released, as I merge the other PRs

@harunnryd
Copy link
Author

Thanks Pak @Nerzal 😊

@Nerzal
Copy link
Owner

Nerzal commented Jan 14, 2022

hmn something seems to be broken in the tests now, not sure if related to the PR itself.
Guess i'll need to check that first

@harunnryd
Copy link
Author

harunnryd commented Jan 14, 2022

I think the problem is in the token, because it's expired Pak @Nerzal

NB: i used Oauth2 Google

@Nerzal
Copy link
Owner

Nerzal commented Jan 24, 2022

Guess we can't put static tokens into the tests, as the pipelines and local tests are going to fail if we do that

@qwertyway
Copy link

Can you please finish this merge request? Really need "subject_issuer" for proper token exchange. Thanks!

@Nerzal
Copy link
Owner

Nerzal commented Apr 7, 2022

I cannot merge this merge request as is.
This is going to break our pipelines, caused by a test that is not working

Nerzal
Nerzal previously approved these changes Oct 18, 2023
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