Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug login problems #252

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

cpfeiffer
Copy link
Contributor

No description provided.

@@ -77,7 +84,12 @@
<appender-ref ref="main"/>
</category>

<category name="br.com.caelum.vraptor.simplemail.aws">
<category name="org.mamute.auth">
<priority value="DEBUG"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be DEBUG by default?

Copy link
Contributor

@felipeweb felipeweb Apr 16, 2016

Choose a reason for hiding this comment

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

@cpfeiffer change here to info and debug

@xdarklight
Copy link
Contributor

I like this change in general - I had some login issues myself due to connection problems to our LDAP server.
Those were really nasty to figure out - so 👍 from me for fixing the log-levels.

@@ -227,7 +228,7 @@ private void updateAvatarImage(LDAPResource ldap, Entry entry, User user) {
private void createUserIfNeeded(LDAPResource ldap, String cn) throws LdapException {
Entry ldapUser = ldap.getUser(cn);
String email = ldap.getAttribute(ldapUser, emailAttr);
User user = users.findByEmail(email);
User user = email != null ? users.findByEmail(email) : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some unit tests to this case that email is null

@felipeweb
Copy link
Contributor

felipeweb commented May 11, 2016

@cpfeiffer can you add the unit tests that I mentioned up there?

@cpfeiffer
Copy link
Contributor Author

Yes, sorry, I will eventually do it, I'm just too busy with other things at the moment. I had to debug login problems at work and needed to fnid out the actual ldap error cause, that's why I quickly updated the patch.

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

Successfully merging this pull request may close these issues.

None yet

3 participants