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

fix: Improve GitHub token validation check #327

Merged
merged 1 commit into from
Jul 13, 2022
Merged

fix: Improve GitHub token validation check #327

merged 1 commit into from
Jul 13, 2022

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Jul 12, 2022

Signed-off-by: Igor Vinokur ivinokur@redhat.com

What does this PR do?

Do not throw an exception when validating an expired oauth token, return false instead.

When a factory starts and oauth token is found, che-server checks the token:

  • current behaiviour: if the oauth token is not valid, the validation check throws an exception and the factory creation is interrupted.
  • PR behaviour: if the oauth token is not valid, the validation check returns false instead of an error, so token regeneration mechanism starts and the factory is created successfully.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes eclipse-che/che#21421

How to test this PR?

  1. Deploy Che with quay.io/ivinokur/che-server:next image e.g. chectl server:deploy -p=minikube -i=quay.io/ivinokur/che-server:next
  2. Configure GitHub oauth
  3. Start a factory from a GitHub repo to generate GitHub token in the user namespace.
  4. Revoke the GitHub token: step 1
  5. Restart the factory.

See: Che will regenerate the token and start the facory.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@vinokurig vinokurig merged commit 24440ea into main Jul 13, 2022
@vinokurig vinokurig deleted the che-21421 branch July 13, 2022 12:17
@che-bot che-bot added this to the 7.51 milestone Jul 13, 2022
@ibuziuk
Copy link
Member

ibuziuk commented Jul 13, 2022

@vinokurig thank you! could you please backport to 7.50.x ?

vinokurig added a commit that referenced this pull request Jul 14, 2022
Do not throw an exception when validating an expired oauth token, return false instead.
When a factory starts and oauth token is found, che-server checks the token:
current behaiviour: if the oauth token is not valid, the validation check throws an exception and the factory creation is interrupted.
PR behaviour: if the oauth token is not valid, the validation check returns false instead of an error, so token regeneration mechanism starts and the factory is created successfully.
vinokurig added a commit that referenced this pull request Jul 14, 2022
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.

Can't start a workspace by factory on dogfooding instance
4 participants