Skip to content
Snippets Groups Projects

Ldap email

Closed Ramya Authappan requested to merge github/fork/oss/ldap_email into master

Created by: katiepru

Added a config option in gitlab.yml.example to set a default email domain in the event that LDAP does not provide email addresses. Currently using this model at Rutgers University. If the "email_domain" option is set and the user does not have an email associated with them already in LDAP, an email address is formulated using the formula "_ldap_nickname@_config_email_domain", which is reflected in auth.rb. This option can be disabled by setting it to null.

Merge request reports

Closed by avatar (Jan 10, 2025 2:28pm UTC)

Merge details

  • The changes were not merged into .

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: katiepru

    Cleaned up code a lot and fixed indentation issues.

  • Created by: raphendyr

    Could you do this using Gitlab.config.omniauth.provider[provider].email_domain or similar? This way this would work for all omniauth providers. I think that would be better than fixing it for every omniauth prvovider.

  • Created by: raphendyr

    I might look into this while implementing pam login.

  • Created by: katiepru

    Sure. I can get that to you on Monday.

  • Created by: raphendyr

    Thanks. I tried this, but didn't get it to work: https://github.com/raphendyr/gitlabhq/commit/925a4e17ae62aef8cc7c14d49e2c984b26144273

  • Created by: raphendyr

    This one works, but looks a lot of bad hack... https://github.com/raphendyr/gitlabhq/commit/c5c9136bb469137f40aae7019b97fe199e78bf9e

  • Created by: katiepru

    Can you explain exactly what you want me to do here? I'm a bit unclear as to what you are asking.

  • Created by: raphendyr

    Well. email might and is missing for other providers (eq. pam) so, while implementing it for ldap It would be good to implement it to all omniauth providers.

    This means I don't think its good idea to use ldap specific setting. Instead you could for example use

    Devise.omniauth_configs[provider].options[:email_domain]

    And that setting can be injected to device in config/initializers/devise.rb

    I don't know if that approach is good or not. If it is I can make that commit a pull request.

    Ps. I'm not in gitlab team. I'm just one trying to implement pam authentication and I shout to all people who are implementing stuff around ldap to make it in more generic way. I do not want to reimplement everything.

    Also if I need to rewrite all the code there none is going to accept my pull-requests. So please thing that there is other authentication methods also.

    Pps. I think this non generic programming is one reason why shibboleth fork is not trying to merge it's code to upstream. Or they have other reason I do not know. I just don't have time to end up keeping pam fork update in same way.

  • Created by: katiepru

    Try submitting that commit as a pull request. If they have any interest in the feature and believe it can be implemented in a better way, someone will tell you. I don't think I am familiar enough with the code base to give you suggestions at this point. It is a good idea though. See if that pull request gets any attention.

  • Created by: jvanbaarsen

    @katiepru What is the status of this PR?

  • Created by: jvanbaarsen

    This pull request has been closed because a request for more information has not been reacted to for more than 2 weeks. If you respond and conform to the pull request guidelines in our contributing guidelines we will reopen this pull request. /cc @Razer6

Please register or sign in to reply
Loading