Clarify LDAP config default values #334

New Issue

2022-07-05T07:53:44Z

pat-s commented

2022-07-05 07:53:44 +00:00

(Migrated from gitea.com)

The Gitea LDAP documentation does not list port, securityProtocol and emailAttribute as required values. Though when omitting these in the helm chart definition, the deployment errors with "XX is not set".

Should the defaults be set to port=389, securityProtocol=unencrypted, emailAttribute=mail?

The [Gitea LDAP documentation](https://docs.gitea.io/en-us/authentication/#ldap-via-binddn) does not list `port`, `securityProtocol` and `emailAttribute` as required values. Though when omitting these in the helm chart definition, the deployment errors with "XX is not set". Should the defaults be set to `port=389`, `securityProtocol=unencrypted`, `emailAttribute=mail`?

justusbunsi commented

2022-07-05 09:41:03 +00:00

(Migrated from gitea.com)

Port and emailAttribute are defined as required in the docs. It's listed in the "shared" properties section above your link. https://docs.gitea.io/en-us/authentication/#ldap-lightweight-directory-access-protocol

Only attribute missing seems to be securityProtocol. But I don't think we should set a default value. Especially not the "unencrypted" value. It should be up to the user to decide whether sending the credentials encrypted or in plaintext. Maybe the Chart could fail during rendering if any of the specified ldap configs is missing a required field. That way applying would already fail and we can give a clear hint on what's the issue.

Port and emailAttribute are defined as required in the docs. It's listed in the "shared" properties section above your link. https://docs.gitea.io/en-us/authentication/#ldap-lightweight-directory-access-protocol Only attribute missing seems to be securityProtocol. But I don't think we should set a default value. Especially not the "unencrypted" value. It should be up to the user to decide whether sending the credentials encrypted or in plaintext. Maybe the Chart could fail during rendering if any of the specified ldap configs is missing a required field. That way applying would already fail and we can give a clear hint on what's the issue.

pat-s commented

2022-07-05 15:14:32 +00:00

(Migrated from gitea.com)

Not in the "via BindDN" option: https://docs.gitea.io/en-us/authentication/#ldap-via-binddn (only looked at this section, you're right they are defined in the others, e.g. https://docs.gitea.io/en-us/authentication/#ldap-lightweight-directory-access-protocol). So it seems they should just be added to this section :)

Only attribute missing seems to be securityProtocol. But I don't think we should set a default value. Especially not the "unencrypted" value. It should be up to the user to decide whether sending the credentials encrypted or in plaintext.

Agree!

Maybe the Chart could fail during rendering if any of the specified ldap configs is missing a required field.

Jup sounds good. It already does but with a Gitea error during container startup which is not so easy to spot.

Not in the "via BindDN" option: https://docs.gitea.io/en-us/authentication/#ldap-via-binddn (only looked at this section, you're right they are defined in the others, e.g. https://docs.gitea.io/en-us/authentication/#ldap-lightweight-directory-access-protocol). So it seems they should just be added to this section :) > Only attribute missing seems to be securityProtocol. But I don't think we should set a default value. Especially not the "unencrypted" value. It should be up to the user to decide whether sending the credentials encrypted or in plaintext. Agree! > Maybe the Chart could fail during rendering if any of the specified ldap configs is missing a required field. Jup sounds good. It already does but with a Gitea error during container startup which is not so easy to spot.

justusbunsi commented

2022-07-05 15:19:42 +00:00

(Migrated from gitea.com)

The first section in the docs is for both ldap config types:

Both the LDAP via BindDN and the simple auth LDAP share the following fields

The second section (via BindDN) just

Adds the following fields

I agree, this could be more obvious.

The first section in the docs is for both ldap config types: > Both the LDAP via BindDN and the simple auth LDAP share the following fields The second section (via BindDN) just > Adds the following fields I agree, this could be more obvious.

pat-s commented

2022-07-06 06:09:15 +00:00

(Migrated from gitea.com)

The first section in the docs is for both ldap config types:

Ah, well spotted!

I agree, this could be more obvious.

Y, I think duplication would not hurt here - or highlighting the shared section more prominently - but I think I 'd favor duplication here.

> The first section in the docs is for both ldap config types: Ah, well spotted! > I agree, this could be more obvious. Y, I think duplication would not hurt here - or highlighting the shared section more prominently - but I think I 'd favor duplication here.

👍 1

szpeter80 commented

2024-01-09 13:42:24 +00:00

(Migrated from gitea.com)

Sorry to revive a such old issue, but as of now, i can not find the possible values of 'securityProtocol'. The only reference is 'unencrypted' in the examples but there is no canonical list of accepted values and their meaning / implication. Am i missing something ?

pat-s commented

2024-01-10 08:26:20 +00:00

(Migrated from gitea.com)

Other values are StartTLS and LDAPS. Documentation at https://docs.gitea.com/next/usage/authentication#ldap-via-binddn should be updated, PRs welcome!

Other values are `StartTLS` and `LDAPS`. Documentation at https://docs.gitea.com/next/usage/authentication#ldap-via-binddn should be updated, PRs welcome!

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#334