[SHIRO-661] Add check for the principal of subject whether is null

[plx927]

When session is based on servlet container(such as tomcat),if the subject is authenticated，the session will contains AUTHENTICATED_SESSION_KEY and PRINCIPALS_SESSION_KEY。
When servlet container closed, it may will be persist session.
But if the principal can not be serializable, it will not be persisted; when server restart, session will only contains AUTHENTICATED_SESSION_KEY info ,the PRINCIPALS_SESSION_KEY will be lost,
it means the subject is authenticated, but the subject does not has principal。If the user code is

User u = subject.getPrincipal();
// because the u if null, it will be npe
u.getName();

Recently, my project has happened such case, so I think add check for principal of subject whether is null can make application more powerful.

[bdemers]

Thanks @plx927!

I'm thinking we should probably update the current Subject impls isAuthenticated method too.

[fpapon]

retest this please

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Shiro-pr/12/

Build result: FAILURE

[...truncated 3.24 MB...][JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/crypto/cipher/pom.xml to org.apache.shiro/shiro-crypto-cipher/1.4.1-SNAPSHOT/shiro-crypto-cipher-1.4.1-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/crypto/cipher/target/shiro-crypto-cipher-1.4.1-SNAPSHOT.jar to org.apache.shiro/shiro-crypto-cipher/1.4.1-SNAPSHOT/shiro-crypto-cipher-1.4.1-SNAPSHOT.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/crypto/cipher/target/shiro-crypto-cipher-1.4.1-SNAPSHOT-javadoc.jar to org.apache.shiro/shiro-crypto-cipher/1.4.1-SNAPSHOT/shiro-crypto-cipher-1.4.1-SNAPSHOT-javadoc.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/crypto/cipher/target/shiro-crypto-cipher-1.4.1-SNAPSHOT-sources.jar to org.apache.shiro/shiro-crypto-cipher/1.4.1-SNAPSHOT/shiro-crypto-cipher-1.4.1-SNAPSHOT-sources.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/samples/web/pom.xml to org.apache.shiro.samples/samples-web/1.4.1-SNAPSHOT/samples-web-1.4.1-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/samples/web/target/samples-web-1.4.1-SNAPSHOT.war to org.apache.shiro.samples/samples-web/1.4.1-SNAPSHOT/samples-web-1.4.1-SNAPSHOT.war[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/samples/web/target/samples-web-1.4.1-SNAPSHOT-sources.jar to org.apache.shiro.samples/samples-web/1.4.1-SNAPSHOT/samples-web-1.4.1-SNAPSHOT-sources.jar[Fast Archiver] Compressed 2.43 MB of artifacts by 64.2% relative to #9[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/servlet-plugin/pom.xml to org.apache.shiro/shiro-servlet-plugin/1.4.1-SNAPSHOT/shiro-servlet-plugin-1.4.1-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/servlet-plugin/target/shiro-servlet-plugin-1.4.1-SNAPSHOT.jar to org.apache.shiro/shiro-servlet-plugin/1.4.1-SNAPSHOT/shiro-servlet-plugin-1.4.1-SNAPSHOT.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/servlet-plugin/target/shiro-servlet-plugin-1.4.1-SNAPSHOT-sources.jar to org.apache.shiro/shiro-servlet-plugin/1.4.1-SNAPSHOT/shiro-servlet-plugin-1.4.1-SNAPSHOT-sources.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/spring/pom.xml to org.apache.shiro/shiro-spring/1.4.1-SNAPSHOT/shiro-spring-1.4.1-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/spring/target/shiro-spring-1.4.1-SNAPSHOT.jar to org.apache.shiro/shiro-spring/1.4.1-SNAPSHOT/shiro-spring-1.4.1-SNAPSHOT.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/spring/target/shiro-spring-1.4.1-SNAPSHOT-javadoc.jar to org.apache.shiro/shiro-spring/1.4.1-SNAPSHOT/shiro-spring-1.4.1-SNAPSHOT-javadoc.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/spring/target/shiro-spring-1.4.1-SNAPSHOT-sources.jar to org.apache.shiro/shiro-spring/1.4.1-SNAPSHOT/shiro-spring-1.4.1-SNAPSHOT-sources.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/guice/pom.xml to org.apache.shiro/shiro-guice/1.4.1-SNAPSHOT/shiro-guice-1.4.1-SNAPSHOT.pom[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/guice/target/shiro-guice-1.4.1-SNAPSHOT.jar to org.apache.shiro/shiro-guice/1.4.1-SNAPSHOT/shiro-guice-1.4.1-SNAPSHOT.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/guice/target/shiro-guice-1.4.1-SNAPSHOT-tests.jar to org.apache.shiro/shiro-guice/1.4.1-SNAPSHOT/shiro-guice-1.4.1-SNAPSHOT-tests.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/guice/target/shiro-guice-1.4.1-SNAPSHOT-javadoc.jar to org.apache.shiro/shiro-guice/1.4.1-SNAPSHOT/shiro-guice-1.4.1-SNAPSHOT-javadoc.jar[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/Shiro-pr/support/guice/target/shiro-guice-1.4.1-SNAPSHOT-sources.jar to org.apache.shiro/shiro-guice/1.4.1-SNAPSHOT/shiro-guice-1.4.1-SNAPSHOT-sources.jarchannel stoppedSetting status of 9c58d30 to FAILURE with url https://builds.apache.org/job/Shiro-pr/12/ and message: 'FAILURE 'Using context: Jenkins: mvn clean install

[fpapon]

retest this please

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Shiro-pr/13/

[fpapon]

Thanks @plx927!

I'm thinking we should probably update the current Subject impls isAuthenticated method too.

@plx927 can you complete your PR with @bdemers remarks please?

[fpapon]

I created a Jira for this:

https://issues.apache.org/jira/browse/SHIRO-661

[fpapon]

@bdemers as we have no update about the user, can I merge this and made the update in the Subject isAuthenticated method in another PR?

[bdemers]

@fpapon i think we would want to figure out how/why a subject is authenticated without a principal.

Possibly making the default isAuthenticated() check for a principal. But i think understanding what happened is our first step

[fpapon]

@bdemers ok, if I understand the use case, this is about the serialization of the Principal. Sometimes it cannot be serialize and became null in the session after restarting the web container.
May be we can check why Principal is not serialized?

[bdemers]

AHH!! maybe adding the null check to isAuthenticated() is the way to go then?

[fpapon]

Yes I think so :)

[fpapon]

I think we could add && hasPrincipals() here:

shiro/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java

Line 297 in 22a2612

return authenticated;

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Shiro-pr/78/