fix: generate email if it is the empty string on external login

[mikeroda]

If the email is the empty string, treat it as null and generate one.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187543759

The labels on this github issue will be updated when the story is started.

[mikeroda]

Without the change, the test case will throw an exception:

java.lang.IllegalArgumentException: Email is required
at org.springframework.util.Assert.hasText(Assert.java:289)
at org.cloudfoundry.identity.uaa.user.UaaUser.(UaaUser.java:152)

Because UaaUser has Assert.hasText(prototype.getEmail(), "Email is required") which is inconsistent with the null check in ExternalLoginAuthenticationManager.

[peterhaochen47]

Why use !StringUtils.hasText(email) here but StringUtils.isEmpty(email) in ExternalLoginAuthenticationManager.java? Why the different logics?

It looks like !StringUtils.hasText is a narrower/stricter validation? It also evaluates to true if the string contains only whitespace(s).

[mikeroda]

Only because this one uses the Spring StringUtils and isEmpty is deprecated. ExternalLoginAuthenticationManager uses the Apache StringUtils. I didn't want to affect other lines of code by changing one or the other, and I didn't want to use a deprecated method.

[peterhaochen47]

Also, what is the issue in real life? When logging in with an external IDP, the email value we get from the IDP would be an empty string & the user will fail to log in due to "Email is required" error? Why would the IDP has an empty string as the email (instead of just not having it)? Is the real issue here the operator didn't set up the attributeMappings config correctly for UAA API?

[mikeroda]

Also, what is the issue in real life? When logging in with an external IDP, the email value we get from the IDP would be an empty string & the user will fail to log in due to "Email is required" error? Why would the IDP has an empty string as the email (instead of just not having it)? Is the real issue here the operator didn't set up the attributeMappings config correctly for UAA API?

The issue is that it's treated inconsistently. A null email will result in one being generated. An empty string will throw an exception that email is required. In reality an attributeMapping for email is not required, that why we can generate one. So we should be consistent about how we do that.