Skip to content

Conversation

@fredowski
Copy link

This is based on the Pull Request #3141 from @jasl8r and Pull Request #3470 from @drahamim. I added the ideas from @randx

So this will add LDAP Authentification in the grack module. With this I can push via https for users which have LDAP or Email authentification. Discussion about this topic was in #2012 and #2557.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.22%) when pulling 228baa8 on fredowski:master into df96c07 on gitlabhq:master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not return false if user.nil?

@dzaporozhets
Copy link
Contributor

@fredowski thanks for this one. I'll merge it if you correct few things

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.61%) when pulling 20a88f5 on fredowski:master into df96c07 on gitlabhq:master.

@fredowski
Copy link
Author

@randx thanks for your remarks. I modified the code according to your comments. In the last commit 20a88f5 I

  • added the check for the enabled ldap before calling ldap_auth
  • added a comment regarding the user.nil? test. user is nil when the previous search for username/email did not succeed.
  • tested again with ldap account, email account, wrong password for https pushes. Here it works.

I hope I did not forget too much. This is my first ruby code...

@dzaporozhets
Copy link
Contributor

@fredowski thank you. Good that you tested it in several cases

dzaporozhets added a commit that referenced this pull request Apr 30, 2013
Authenticate LDAP users in the grack module - fixed problems - tested code
@dzaporozhets dzaporozhets merged commit 7ec1cfd into gitlabhq:master Apr 30, 2013
@ghost ghost assigned dzaporozhets Apr 30, 2013
stanhu pushed a commit that referenced this pull request Dec 7, 2015
rspeicher pushed a commit that referenced this pull request Dec 7, 2015
Fix #3758: Serious performance issues due to timeago()
being called n*(n+1)/2 times instead of n

See bug #3758 for a description. This merge request alters 
`time_ago_with_tooltip` to invoke the `timeago()` javascript on the
current timestamp only, instead of each one defined on the page so far.

See merge request !1977
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants