Issuer doesn't always have saml/metadata

[davidgomes]

I am running into an issue that is very similar to #233 by @omidgz.

I am having to do:

samlSP.ServiceProvider.MetadataURL = ...

After creating the service provider because saml/metadata is being appended to the issuer field. However, it doesn't seem like this is always the case.

I'm using a fairly popular Docker IP image for SAML 2.0 testing:

https://hub.docker.com/r/kristophjunge/test-saml-idp/

When using this image, the issuer field has to be exactly <saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://app.example.com</saml:Issuer>.

I'm not sure what the fix to crewjam/saml looks like, but it seems like there should be an option to not add saml/metadata to whatever is passed into the issuer field.

[crewjam]

Thanks for the report!

Hmm, I haven't seen that before. In the absense of evidence to the contrary, my inclination is that assigning your own metadata URL is a pretty reasonable thing, if you need it for some reason. Is that so terrible?

Perhaps the other issue is that we are (as a matter of simplicity) conflating the MetadataURL with the Issuer. I thought that was okay because I thought we, as the SP, could determine the issuer. here

If that isn't the case, perhaps the way to address this is to expose the issuer as a separate setting with a default?

It would be helpful to see a slightly larger block of code from your usage to figure out how severe of an issue this is and what the fix might be.

[davidgomes]

Hmm, I haven't seen that before. In the absense of evidence to the contrary, my inclination is that assigning your own metadata URL is a pretty reasonable thing, if you need it for some reason. Is that so terrible?

I think that's a fair assumption - I certainly don't mind having to set it myself (maybe we should just document it).

As for the conflating issue, I don't believe I'm qualified to comment on it since I don't really understand those SAML concepts very well yet.

Anyhow, if it helps, here's a larger block of code of my usage of this library:

		samlSP, _ := samlsp.New(samlsp.Options{
			URL:            *rootURL,
			IDPMetadataURL: idpMetadataURL,
			Key:            keyPair.PrivateKey.(*rsa.PrivateKey),
			Certificate:    keyPair.Leaf,
		})

		// https://github.com/crewjam/saml/issues/238
		samlSP.ServiceProvider.MetadataURL = *rootURL
		AcsURL, _ := url.Parse("http://localhost:8080/api/saml-assert")
		samlSP.ServiceProvider.AcsURL = *AcsURL

		samlSP.RequireAccountHandler(w, r)

		redirectURL, err := url.Parse(w.Header().Get("Location"))
		if err != nil {
			http.Error(w, fmt.Sprintf("Parsing the Redirect URL header failed: %s", err), http.StatusBadRequest)
			return
		}

		relayState, ok := redirectURL.Query()["RelayState"]
		if !ok || len(relayState[0]) < 1 {
			http.Error(w, fmt.Sprintf("Parsing the relay state in the Redirect URL header failed"), http.StatusBadRequest)
			return
		}

		requestID := relayState[0]

		samlRequests[requestID] = samlLogin{
			clusterID: clusterID[0],
			user:      user[0],
			samlSP:    samlSP,
		}

[adenix]

This looks like it's fixed by #258 - commenting for those evaluating the library for use