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 revoke authorisation implementation for Gitlab #667

Merged
merged 1 commit into from
Mar 16, 2024
Merged

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Mar 12, 2024

What does this PR do?

Add revoke authorisation implementation for Gitlab.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#22832

How to test this PR?

  1. Deploy Che from the PR image: quay.io/eclipse/che-server:pr-667.
  2. Apply dashboard image from the related PR: quay.io/eclipse/che-dashboard:pr-1068.
  3. Setup gitlab oauth for onprem or SAAS version.
  4. Start a workspace from the gitlab repository with a devfile, accept the authentication request.
  5. Go to dashboard -> User Preferences -> Git Services tab and revoke the gitlab authorisation.
  6. Delete the workspace.
  7. Start the workspace again.

See: The authorisation request appears as it was rejected by the revoke action.

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.

@tolusha
Copy link
Contributor

tolusha commented Mar 12, 2024

Could you share api documentation?

@vinokurig
Copy link
Contributor Author

/retest

@vinokurig vinokurig changed the title Add Gitlab invalidate token implementation Add revoke authorisation implementation for Gitlab Mar 12, 2024
@vinokurig
Copy link
Contributor Author

/retest

1 similar comment
@vinokurig
Copy link
Contributor Author

/retest

@vinokurig
Copy link
Contributor Author

@tolusha

Could you share api documentation?

https://docs.gitlab.com/ee/api/oauth2.html#revoke-a-token

try {
HttpResponse<InputStream> response =
client.send(request, HttpResponse.BodyHandlers.ofInputStream());
return response.statusCode() == 200;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it return if token not found (already removed) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can not remove an oauth token from Gitlab. If we manually revoke the authorisation from Gitlab, and then execute the revoke action from che dashboard, we will have status code 200.

@openshift-ci openshift-ci bot added the lgtm label Mar 15, 2024
@dmytro-ndp dmytro-ndp self-requested a review March 15, 2024 10:58
Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

Tested successfully following test scenario from PR description using GitLab Server.
Screencast:
screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.03.16-02_00_29.webm

Additional verification has passed as well: no authorization request appeared on new workspace start if not revoke gitlab authorization.
Screencast:
screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.03.15-17_27_56.webm

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

I have noticed later, that if revoke authorization and refresh "Git Services" page, it shows "Authorized" status again:
screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.03.16-02_09_36.webm

@vinokurig : could you please take a look?

@openshift-ci openshift-ci bot removed the lgtm label Mar 16, 2024
@vinokurig
Copy link
Contributor Author

vinokurig commented Mar 16, 2024

I have noticed later, that if revoke authorization and refresh "Git Services" page, it shows "Authorized" status again:
screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.03.16-02_09_36.webm

@dmytro-ndp That's a known issue: eclipse-che/che#22790

@dmytro-ndp
Copy link
Contributor

@vinokurig : I see.
Thanks for the explanation.

@openshift-ci openshift-ci bot added the lgtm label Mar 16, 2024
Copy link

openshift-ci bot commented Mar 16, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dmytro-ndp, tolusha, vinokurig

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vinokurig vinokurig merged commit ba6f30f into main Mar 16, 2024
28 checks passed
@vinokurig vinokurig deleted the che-22832 branch March 16, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants