Account autocreation from LDAP after reverse proxy authentication #18452

pboguslawski · 2022-01-29T16:39:46Z

Gitea allows autocreation of account from external source after successful
basic auth but not after successful reverse proxy auth. This mod adds such
feature.

It also adds full name, email and is_active synchronization from LDAP on
user login (as cron task already does).

Related: gogs/gogs#2498
Author-Change-Id: IB#1104925

Gitea allows autocreation of account from external source after successful basic auth but not after successful reverse proxy auth. This mod adds such feature. Unfortunaltely gitea does not sync all user attributes from LDAP for existing users on login like cron.sync_external_users does so changes of first name, surname, e-mail are not updated from LDAP on login for exiting users - only after first login and after sync_external_users task. Related: gogs/gogs#2498 Author-Change-Id: IB#1104925

pboguslawski · 2022-01-29T16:42:19Z

This is simplified and adjusted for current main version of #12960.

pboguslawski · 2022-01-29T17:35:50Z

This is quick and dirty workaround for installations using reverse proxy auth with ldap user data backend.

More elegant fixes will probably require gitea to separate auth/sync areas better like in
#18453

pboguslawski · 2022-02-01T11:34:05Z

If #18466 is accepted, than should be merged first, than this mod must use UpdateForceUserCols instead of UpdateUserCols.

Gitea should synchronize same data from LDAP on user authentication that is synchronized in cron task. Fixes: a5c21f1 Related: go-gitea#18452 Author-Change-Id: IB#1104925

pboguslawski · 2022-02-04T09:39:32Z

After ca00ec6 gitea is able also to reactivate user account on login if enabled in LDAP (without it user had to wait for cron task to unlock gitea account). Fullname and email are synchronized now on login also (like in cron task).

stale · 2022-04-17T13:06:56Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

`SessionUser` should be protected against passing `sess` = `nil` to avoid ``` PANIC: runtime error: invalid memory address or nil pointer dereference ``` in https://github.com/go-gitea/gitea/pull/18452/files#diff-a215b82aadeb8b4c4632fcf31215dd421f804eb1c0137ec6721b980136e4442aR69 after upgrade from gitea v1.16 to v1.17. Related: go-gitea#18452 Author-Change-Id: IB#1126459

`SessionUser` should be protected against passing `sess` = `nil` to avoid ``` PANIC: runtime error: invalid memory address or nil pointer dereference ``` in https://github.com/go-gitea/gitea/pull/18452/files#diff-a215b82aadeb8b4c4632fcf31215dd421f804eb1c0137ec6721b980136e4442aR69 after upgrade from gitea v1.16 to v1.17. Related: #18452 Author-Change-Id: IB#1126459

`SessionUser` should be protected against passing `sess` = `nil` to avoid. ``` PANIC: runtime error: invalid memory address or nil pointer dereference ``` in https://github.com/go-gitea/gitea/pull/18452/files#diff-a215b82aadeb8b4c4632fcf31215dd421f804eb1c0137ec6721b980136e4442aR69 after upgrade from gitea v1.16 to v1.17. Related: go-gitea#18452 Author-Change-Id: IB#1126459

Fixes: adac68d Related: go-gitea#21358 Related: go-gitea#18452 Author-Change-Id: IB#1126459

Backport #21358 `SessionUser` should be protected against passing `sess` = `nil` to avoid ``` PANIC: runtime error: invalid memory address or nil pointer dereference ``` in https://github.com/go-gitea/gitea/pull/18452/files#diff-a215b82aadeb8b4c4632fcf31215dd421f804eb1c0137ec6721b980136e4442aR69 after upgrade from gitea v1.16 to v1.17. Related: #18452

denyskon · 2023-08-02T13:35:33Z

@pboguslawski What is the state of this PR?

pboguslawski · 2023-09-08T10:08:14Z

What is the state of this PR?

The status of this PR is Open for over year now as you can see.

We use this extension and it works fine for us. Let us know if you want to merge it - will update to current main then.

KN4CK3R · 2023-09-08T18:17:50Z

services/auth/reverseproxy.go

+	// Just return user if session is estabilshed already.
+	user := SessionUser(sess)
+	if user != nil {
+		return user
+	}


This mixes the Session provider into the reverse proxy. I think we should not do this. Is it needed for the new logic?

This mixes the Session provider into the reverse proxy. I think we should not do this. Is it needed for the new logic?

Reverse proxy is/should be for user authentication stuff not Gitea's session handling IHMO. So there is no point in user verification if already verified (established session). Don't see problem with it.

Then it's just a wrong execution order and the session provider should be checked first.

SessionUser(sess) is called first and used if session is already estabilshed; if no session is established, new session is created. Don't see any problem here. If you see problem here, please propose exact code fix.

The session handler is already called before the reverse proxy handler:

gitea/routers/web/web.go

Lines 104 to 111 in cb10f27

group := auth_service.NewGroup(

&auth_service.OAuth2{}, // FIXME: this should be removed and only applied in download and oauth related routers

&auth_service.Basic{}, // FIXME: this should be removed and only applied in download and git/lfs routers

&auth_service.Session{},

)

if setting.Service.EnableReverseProxyAuth {

group.Add(&auth_service.ReverseProxy{})

}

Fixed in dfc544c. Thanks for pointing.

Fixes: a5c21f1 Related: go-gitea#18452 (comment) Author-Change-Id: IB#1104925 Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>

pboguslawski mentioned this pull request Jan 29, 2022

Account autocreation from LDAP after reverse proxy authentication #12960

Closed

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 29, 2022

pboguslawski mentioned this pull request Jan 29, 2022

Further separation of authentication and synchronization areas in sources #18453

Open

Added missing data synchronization after LDAP auth

ca00ec6

Gitea should synchronize same data from LDAP on user authentication that is synchronized in cron task. Fixes: a5c21f1 Related: go-gitea#18452 Author-Change-Id: IB#1104925

pboguslawski mentioned this pull request Feb 4, 2022

Inconsistent e-mail address sync from LDAP #18603

Closed

stale bot added the issue/stale label Apr 17, 2022

lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Apr 18, 2022

stale bot removed the issue/stale label Apr 18, 2022

pboguslawski mentioned this pull request Oct 6, 2022

SessionUser protection against nil pointer dereference #21358

Merged

pboguslawski added a commit to ibpl/gitea that referenced this pull request Oct 24, 2022

Verification moved to correct method

09628bb

Fixes: adac68d Related: go-gitea#21358 Related: go-gitea#18452 Author-Change-Id: IB#1126459

zeripath mentioned this pull request Oct 24, 2022

SessionUser protection against nil pointer dereference #21581

Merged

denyskon added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Aug 2, 2023

techknowlogick removed the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Sep 8, 2023

KN4CK3R reviewed Sep 8, 2023

View reviewed changes

denyskon added type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented labels Sep 10, 2023

Josue-T mentioned this pull request Jan 24, 2024

LDAP "Admin Filter" stopped working #25985

Closed

Josue-T mentioned this pull request Feb 6, 2024

Manifestv2 improvements YunoHost-Apps/gitea_ynh#93

Merged

2 tasks

Remove unnecessary session verification

dfc544c

Fixes: a5c21f1 Related: go-gitea#18452 (comment) Author-Change-Id: IB#1104925 Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 28, 2024

github-actions bot added the modifies/go Pull requests that update Go code label Mar 28, 2024

denyskon requested a review from KN4CK3R July 9, 2024 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account autocreation from LDAP after reverse proxy authentication #18452

Account autocreation from LDAP after reverse proxy authentication #18452

pboguslawski commented Jan 29, 2022 •

edited

Loading

pboguslawski commented Jan 29, 2022

pboguslawski commented Jan 29, 2022

pboguslawski commented Feb 1, 2022

pboguslawski commented Feb 4, 2022

stale bot commented Apr 17, 2022

denyskon commented Aug 2, 2023

pboguslawski commented Sep 8, 2023 •

edited

Loading

KN4CK3R Sep 8, 2023

pboguslawski Dec 23, 2023

KN4CK3R Dec 28, 2023

pboguslawski Dec 30, 2023

KN4CK3R Dec 30, 2023

pboguslawski Mar 28, 2024

	group := auth_service.NewGroup(
	&auth_service.OAuth2{}, // FIXME: this should be removed and only applied in download and oauth related routers
	&auth_service.Basic{}, // FIXME: this should be removed and only applied in download and git/lfs routers
	&auth_service.Session{},
	)
	if setting.Service.EnableReverseProxyAuth {
	group.Add(&auth_service.ReverseProxy{})
	}

Account autocreation from LDAP after reverse proxy authentication #18452

Are you sure you want to change the base?

Account autocreation from LDAP after reverse proxy authentication #18452

Conversation

pboguslawski commented Jan 29, 2022 • edited Loading

pboguslawski commented Jan 29, 2022

pboguslawski commented Jan 29, 2022

pboguslawski commented Feb 1, 2022

pboguslawski commented Feb 4, 2022

stale bot commented Apr 17, 2022

denyskon commented Aug 2, 2023

pboguslawski commented Sep 8, 2023 • edited Loading

KN4CK3R Sep 8, 2023

Choose a reason for hiding this comment

pboguslawski Dec 23, 2023

Choose a reason for hiding this comment

KN4CK3R Dec 28, 2023

Choose a reason for hiding this comment

pboguslawski Dec 30, 2023

Choose a reason for hiding this comment

KN4CK3R Dec 30, 2023

Choose a reason for hiding this comment

pboguslawski Mar 28, 2024

Choose a reason for hiding this comment

pboguslawski commented Jan 29, 2022 •

edited

Loading

pboguslawski commented Sep 8, 2023 •

edited

Loading