java-webauthn-server icon indicating copy to clipboard operation
java-webauthn-server copied to clipboard

Building null origins in RelyingParty

Open philsmart opened this issue 1 year ago • 6 comments

I think you should be able to build the RelyingParty object using a null Origins Set (it just takes the RP ID if it is). However, the builder will not allow it Caused by: java.lang.NullPointerException: origins is marked non-null but is null. I guess that is because of the nonnull annotation on the origins field.

Version 2.5.0.

philsmart avatar Feb 02 '24 14:02 philsmart

Sorry, I don't understand what you mean by this:

(it just takes the RP ID if it is)

What are you trying to accomplish?

emlun avatar Feb 02 '24 14:02 emlun

If I want to build the RelyingParty object e.g.

final RelyingParty rp = RelyingParty.builder().identity(
                    RelyingPartyIdentity
                    .builder()
                    .id(getRelyingPartyId())
                    .name(getRelyingPartyName())
                    .build()).credentialRepository(getCredentialRepository())
            .allowOriginPort(isAllowOriginPort())
            .allowOriginSubdomain(isAllowOriginSubdomain())
            .origins(getOrigins())
            .build();

If getOrigins is null, the builder fails with an NPE. I want to pass null through the builder, the constructor seems to allow null origins, and if it is null it sets the identity.getId()

    this.origins =
        origins != null
            ? CollectionUtil.immutableSet(origins)
            : Collections.singleton("https://" + identity.getId());

Otherwise, I have to build the Relying Party object conditionally.

philsmart avatar Feb 02 '24 14:02 philsmart

What you're asking for is in fact the default behaviour, i.e., what you get if you do not call the .origins() setter:

public RelyingParty.RelyingPartyBuilder origins(@nonnull @nonnull Set<String> origins) [...] The default is the set containing only the string "https://" + RelyingParty.getIdentity().getId(). [...]

Does that solve your problem?

emlun avatar Feb 02 '24 15:02 emlun

Thanks for the quick reply. Yes, I am currently handling it conditionally, e.g. if the origins are not set (configured on my service) I just leave it out, and if it is set I call it. It would be easier not to make that decision when I am building the RelyingParty object, and just send through a null and let the builder handle it.

But I get it is a convenience thing, so feel free to close this issue if it is not something you'd want to change.

philsmart avatar Feb 02 '24 16:02 philsmart

Fair enough, I had the same thought while looking into this. We'll consider it for the next release!

emlun avatar Feb 02 '24 20:02 emlun

Thank you.

philsmart avatar Feb 05 '24 09:02 philsmart