Skip to content

Conversation

@lcgkm
Copy link

@lcgkm lcgkm commented Nov 6, 2025

We identified an issue with OAuth2 synchronization. The following logs indicate the problem:

Nov 06 01:05:45 ecs-gitea gitea[629433]: 2025/11/06 01:05:45 .../oauth2/source_sync.go:50:(*Source).refresh() [T] Syncing login_source_id=1 external_id=00u1wwyot5yExxxxxd8 expiration=2025-11-05 16:55:33 +0800 CST
Nov 06 01:05:45 ecs-gitea gitea[629433]: 2025/11/06 01:05:45 .../oauth2/source_sync.go:80:(*Source).refresh() [I] SyncExternalUsers[Okta] disabling user 0

The logs show that the gitea correctly identified the user with external ID 00u1wwyot5yExxxxxd8 for deactivation. However, the action was incorrectly applied to user ID 0 instead.

So, we'd better to use ID to identify a user, not LoginName. For Okta, the ExternalID is Okta User ObjectID, LoginName!= ExternalID.

#31572

We'd better to use id to find a user. Not LoginName. And LoginName!= ExternalID
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 6, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Nov 6, 2025
@lcgkm
Copy link
Author

lcgkm commented Nov 6, 2025

cc @bohde @6543 @kdumontnu

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 6, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 6, 2025

The logs show that the gitea correctly identified the user with external ID 00u1wwyot5yExxxxxd8 for deactivation. However, the action was incorrectly applied to user ID 0 instead.

It seems to be a log-only problem. It shouldn't output the user.ID into log because it is not used or set.

So, we'd better to use ID to identify a user, not LoginName. For Okta, the LoginName is Okta User ObjectID, LoginName!= ExternalID.

I don't think it is necessary.

In Gitea: User.LoginName = ExternalLoginUser.ExternalID = OAuth2 Provider's UserID (id)

If you think here it is wrong, then there must be more places which are also wrong.

@wxiaoguang wxiaoguang marked this pull request as draft November 6, 2025 05:18
@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Nov 6, 2025
@lcgkm
Copy link
Author

lcgkm commented Nov 6, 2025

Hi @wxiaoguang it is not a log-only problem. Let me show you what I got from our db:
screenshot-20251106-134011

You can find the user 'Okta Support'(User ID is 19), his external_id is "00u1wwyot5yExxxxxd8", but his login_name is "okta". So, I think the LoginName!= ExternalID.

In Gitea: User.LoginName = ExternalLoginUser.ExternalID = OAuth2 Provider's UserID (id)
If you think here it is wrong, then there must be more places which are also wrong.

Actually, you can reproduce this issue as your wish. Just create a local account, and link it to oauth2 authn source.
PS. We set ENABLE_AUTO_REGISTRATION=false for oauth2_client.

Finally, In Gitea: User.ID = ExternalLoginUser.UserID, this is what I have confirmed!

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 6, 2025

You can find the user 'Okta Support'(User ID is 19), his external_id is "00u1wwyot5yExxxxxd8", but his login_name is "okta". So, I think the LoginName!= ExternalID.

In Gitea: User.LoginName = ExternalLoginUser.ExternalID = OAuth2 Provider's UserID (id)
If you think here it is wrong, then there must be more places which are also wrong.

Actually, you can reproduce this issue as your wish. Just create a local account, and link it to oauth2 authn source. PS. We set ENABLE_AUTO_REGISTRATION=false for oauth2_client.

Then there must be more wrong places in code which need to be fixed.

Why and how but his login_name is "okta". could happen .......

AFAIK, User.LoginName = ExternalLoginUser.ExternalID, they should all use the OAuth2 Provider's gothUser.UserID. If here something mismatched, then there must be some root problems need to figure out.

Code screenshot:

image image

@lcgkm
Copy link
Author

lcgkm commented Nov 6, 2025

Hi @wxiaoguang

Then there must be more wrong places in code which need to be fixed.

While I am not a Gitea expert, maybe other people can provide other PRs for this.
But for oauth2 sync issue, based on the evidence, we are confident that this fix addresses the root cause of this issue. I would greatly appreciate your review and approval of this pull request.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 6, 2025

We would greatly appreciate your review and approval of this pull request.

Usually I don't approve an unclear change. If it is really unclear and unable to figure out, it needs enough comments and tests to document the problem.

While I am not a Gitea expert, maybe other people can provide other PRs for this.

So as the comment above #35875 (comment): cc @bohde @6543 @kdumontnu

@lcgkm
Copy link
Author

lcgkm commented Nov 6, 2025

BTW, my gitea version is 1.24.3.

And I have found another issue about 'expires_at' for FindExternalUserOptions in external_login_user.go
https://github.com/go-gitea/gitea/blob/main/models/user/external_login_user.go#L202

if opts.Expired {
		cond = cond.And(builder.Lt{"expires_at": time.Now()})
	}

The SQL query log show me that the expires_at in cond used UTC as timezone, but we have set CST in DB. There is timezone mismatch caused the oauth2 sync will delay 8 hours.

This is our DB query log:
11 Execute SELECT external_id, user_id, login_source_id, raw_data, provider, email, name, first_name, last_name, nick_name, description, avatar_url, location, access_token, access_token_secret, refresh_token, expires_at FROM external_login_user WHERE expires_at<'2025-11-05 10:46:02.181994211' AND refresh_token<>'' AND login_source_id=1 LIMIT 50

'2025-11-05 10:46:02.181994211' is UTC, and the Expired is always "false".

PS. I don't know how to fix it. So I just provide the details of the issue here. Or I can submit a new issue about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants