repman-io / repman

Repman - PHP Repository Manager: packagist proxy and host for private packages
https://repman.io
MIT License
519 stars 107 forks source link

Fix gitlab v15 token refresh #596

Open KSauter opened 2 years ago

codecov[bot] commented 2 years ago

Codecov Report

Merging #596 (32c0d24) into master (50fe808) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #596   +/-   ##
=========================================
  Coverage     99.16%   99.16%           
- Complexity     1910     1912    +2     
=========================================
  Files           301      301           
  Lines          6072     6078    +6     
=========================================
+ Hits           6021     6027    +6     
  Misses           51       51           
Impacted Files Coverage Δ
src/Controller/OAuth/GitLabController.php 100.00% <100.00%> (ø)
src/Entity/User/OAuthToken.php 100.00% <100.00%> (ø)
src/Service/User/UserOAuthTokenRefresher.php 100.00% <100.00%> (ø)
...rvice/User/UserOAuthTokenRefresher/AccessToken.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 50fe808...32c0d24. Read the comment docs.

KSauter commented 2 years ago

:warning: This will not fix already broken entries, or expired tokens which have no expire date in the database.

For this, an additional migrate task would be required.

Fahl-Design commented 2 years ago

interesting find and fix, is creating/register a new user from "oauth" still possible? I will check it tomorrow

belprixx commented 2 years ago

:up: please

slappyslap commented 2 years ago

Hi, This PR works great, we just need to delete the record for gitlab in "user_oauth_token" table in the database

babbassp commented 2 years ago

+1. Updated code with these changes and I'm able to synchronize packages again. Thanks @KSauter and @Fahl-Design for addressing this!

[edit] Spoke too soon. Still gives a bad request...

temp commented 2 years ago

Stumbled upon this PR after I got errors in repman after upgrading to gitlab 15.1. Applied the changes, emtpied the user_oauth_token table, and it worked like a charm. However, today it is back to errors on updates, with a different error:

Error: An error occurred while refreshing the access token: Bad Request

Anyone else experiencing this?

temp commented 2 years ago

The error from gitlab is:

{"error":"invalid_grant","error_description":"The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client."}
xvilo commented 2 years ago

We've just upgraded our GitLab to 15.x as well and are also seeing these issues. Yesterday I was able to fix it by adding an expires timestamp (might want to set a date by default in the next version). But are also now seeing the bad request error.

@temp / @KSauter / @babbassp do we need more work on this PR to get everything working? Or can we see if this one can be merged already, and create a new PR for other issues regarding the token refreshing for GitLab?

temp commented 2 years ago

@xvilo like I stated in this thread (or another one? don't know), the PR works, but for us it still requires manual intervention. Need to clear the user-token table, call the "add repo" page, afterwards I can synchronize packages for a few minutes. So it seems to me that something is still missing...

KSauter commented 2 years ago

@temp are you sure that you are running the newest version of this MR? It works for me since Jun without any manual steps.

@xvilo if you set the expire date before you applied this patch, the refreshed tokens in your databare are broken. Please test if applying this patch and deleting the tokens from the DB will help. You have to go to the "app package" => "Gitlab" process to generate a new token after you updated everyting.

temp commented 2 years ago

@KSauter doh. I really had an error in the changed files. Will report later if it works now. But one thing is still weird, I deleted everything from user_oauth_token table, afterwards logged in again, and I wasn't able to sync, because of "Missing OAuth Token". Had to jump to add package and select gitlab, afterwards the sync worked. Shouldn't the OAuth token be fetched automatically?

xvilo commented 2 years ago

So, to add, on 31 Aug 2022 we started using this branch (custom deployed fork on Kubernetes). Which, after removing the OAuth tokens from DB worked fine again.

HOWEVER, I'm currently looking at it (around 2 days later) and seeing these again: Screenshot 2022-09-02 at 15 02 07

I suspect this PR is not fully ready yet, so this might confirm @temp's suspicions

slappyslap commented 2 years ago

Hey, when i reboot all the virtual machine of repman (Installation with ansible) the error with token will apear an another time, and i need to clear the user_oauth_token tabke again and retry the authentification with gitlab and it work again

f3l1x commented 1 year ago

Confirmed @slappyslap comment.

  1. DELETE FROM "public"."user_oauth_token" WHERE "id" = 'yourtokenid';
  2. Go to organization -> add package -> gitlab and authenticate again.
  3. Works.
mv-ics commented 1 year ago

@f3l1x Thx for the workaround. For people like me, running this in docker, there is a simple way to run the query using the console:

bin/console doctrine:query:sql "<QUERY HERE>"
mikk150 commented 1 year ago

Seems to work fine for 55 days

xuandung38 commented 1 year ago

Any update ?

prey87 commented 1 year ago

Any update ?

slappyslap commented 1 year ago

I still have a problem, when i reboot the VM (ansible install), i need to clear the token and reconnect to gitlab every reboot, but i'm not able to reproduce this locally on my mac, I'm going to investigate with clone of the VM

xvilo commented 1 year ago

Hi folks, how do we get this merged? /cc @akondas

moay commented 1 year ago

This bug is awfully annoying. We keep running into this on an near to every day basis. Is there any way we can speed this up here?

Does this PR work with the current code base? If so, why can't we merge it?

xvilo commented 1 year ago

AFAIK @akondas is the only one with write access, and he seems to have stopped responding on any issue or PR.

We can create a fork and apply the patches ourselves, but this will only help with self-hosted instances of Repman

konanado commented 1 year ago

Nice project, but we are using Packeton with GitLab synchronization and merge request review feature. But it would be nice to have this features here too.

slappyslap commented 1 year ago

We plan to switch to packeton too, any tips for me ?

Le ven. 28 juil. 2023 à 16:01, konanado @.***> a écrit :

Nice project, but we are using Packeton https://github.com/vtsykun/packeton with GitLab synchronization and merge request review feature. But it would be nice to have this features here too.

— Reply to this email directly, view it on GitHub https://github.com/repman-io/repman/pull/596#issuecomment-1655743332, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTZEPZSVIKZ44E553ARI3DXSPA4ZANCNFSM5ZJPSIOA . You are receiving this because you were mentioned.Message ID: @.***>

xvilo commented 1 year ago

It seems Packaton is not easy to scale horizontally, as there is no option to save package dists to S3, they need to be on disk. So doing this, requiring something like NFS shares or ReadWriteMany PVC volumes on Kubernetes. In case you don't need to scale that way, it's not such of a big issue, but for us it might be

xvilo commented 1 year ago

It's really weird, it did work for quite a while. But since a week or two (no update on GitLab's side) it's not working anymore and we manually have to create a new token. I'm a bit confused by this TBH

mv-ics commented 10 months ago

Just to leave this here: Packeton is really easy to setup. We have a setup where we run the docker version behind Caddy as a reverse proxy. Took me round about 2 hours to figure everything out + get it running. Sad to see Repman go, but it just got too annoying to revive the token every 2 days.

Sevyls commented 7 months ago

any update?

xvilo commented 4 days ago

I'd like to point out that: