RFR: 8282730: LdapLoginModule throw NPE from logout method after login failure

[Sean Mullan]

On Fri, 1 Jul 2022 17:31:06 GMT, Weijun Wang <weijun at openjdk.org> wrote:

> Add null-checks in all `LoginModule` implementations. It's possible that an application calls `logout` after a login failure, where most internal variables for principals and credentials are null and removing a null from the `Subject`'s principals and credentials sets will trigger a `NullPointerException`.

Changes requested by mullan (Reviewer).

src/java.base/share/classes/javax/security/auth/spi/LoginModule.java line 231:

> 229:      *      before removing it from the Principals or Credentials set
> 230:      *      of a {@code Subject}, otherwise a {@code NullPointerException} will
> 231:      *      be thrown. This is especially important when this method is called

I might also add: "... will be thrown as these sets prohibit null elements." Even better, you could add a hyperlink "these sets prohibit null elements" to the `Subject` constructor which states this.

Also, I recommend updating the `Subject` constructors and clarifying that removing null elements also causes an NPE. Basically update the following sentence (change in italics):

These Sets also prohibit null elements, and attempts to add, *query, or remove* a null element will result in a NullPointerException.

src/java.base/share/classes/javax/security/auth/spi/LoginModule.java line 231:

> 229:      *      before removing it from the Principals or Credentials set
> 230:      *      of a {@code Subject}, otherwise a {@code NullPointerException} will
> 231:      *      be thrown. This is especially important when this method is called

s/when/if/

test/jdk/javax/security/auth/login/modules/SafeLogout.java line 37:

> 35:  * @bug 8282730
> 36:  * @key randomness
> 37:  * @summary LdapLoginModule throw NPE from logout method after login failure

I think the summary can be more descriptive here and doesn't have to match the bug description. How about "Check that all LoginModule implementations don't throw NPE from logout method after login failure"

test/jdk/javax/security/auth/login/modules/SafeLogout.java line 51:

> 49: 
> 50:     static void test(int pos) throws Exception {
> 51:         // Create random JAAS login config.

I'm probably missing something obvious, but can you explain why this test uses a random login config? I would add some comments explaining that more.

-------------

PR: https://git.openjdk.org/jdk/pull/9348