Return parsed values of AuthnRequest to get attributes like ForceAuthn

[mayankagwl]

Describe the bug ForceAuthn attribute used to perform the re-authentication of the user if the user already login Currently IDP.ValidateAuthnRequest function returns the error if any, to get to know the value of the attributes like Id andForceAuthn as well

[avinashdhinwa]

@mayankagwl I would like to pick this up, can you please point me to some resources so that I can understand this issue well

[mayankagwl]

@avinashdhinwa Currently, while parsing the AuthnRequest we are just returning the validation error

validationError := idp.ValidateAuthnRequest(method"POST",query url.Values,payload url.Values);
if validationError !=nil {
  return validationError
}

Instead of returning the only the validation error, we would like to return the parsed value of AuthnRequest as well like: ForceAuthn, ID, IsPassive, Issuer, AuthnContextClassRef etc Above all value come with the AuthnRequest

Using these values implementer can perform the action. ex. if ForceAuthn is true, then the user needs to reauthenticate if already logged in.

So we just need to return the parsed value of AuthnRequest Thanks

[avinashdhinwa]

@mayankagwl Understood the problem.

One more question, is the structure of AuthnRequest documented somewhere? I could see the file utility/request.go which creates a dummy request and has two keys RelayState and SAMLRequest (base64 encoded data), so will all the attributes be present in SAMLRequest or there be more keys in the request itself. Can you point me to some kind of documentation of these request structure, its possible that this is a global standard but I'm not aware of it.

Thanks

[mayankagwl]

@avinashdhinwa You can take below an example of AuthnRequest

<saml2p:AuthnRequest xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol"
                     AssertionConsumerServiceURL="https://service-provider.com/saml/post"
                     Destination="https://identity-provider.com/saml/post"
                     ForceAuthn="true"
                     ID="SNC9de275f658e0b31c1dd9b8a3160f94e9"
                     IsPassive="false"
                     IssueInstant="2020-10-01T12:37:33.128Z"
                     ProviderName="SP test"
                     ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                     Version="2.0">
  <saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">https://service-provider.com</saml2:Issuer>
  <saml2p:NameIDPolicy AllowCreate="true"
                       Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" />
  <saml2p:RequestedAuthnContext Comparison="exact">
    <saml2:AuthnContextClassRef xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml2:AuthnContextClassRef>
  </saml2p:RequestedAuthnContext>
</saml2p:AuthnRequest>

[avinashdhinwa]

Thanks @mayankagwl , this is helpful

[avinashdhinwa]

@mayankagwl I need to make changes to AuthnRequest struct in internal/schema.go to accommodate these new fields, Right?

[avinashdhinwa]

@mayankagwl Since the schema of AuthnRequest is defined in an internal package, how do we send AuthRequest data to users of this library? Should we create another struct in saml package which is similar to AuthnRequest or should the library return an interface

[avinashdhinwa]

@mayankagwl Since the schema of AuthnRequest is defined in an internal package, how do we send AuthRequest data to users of this library? Should we create another struct in saml package which is similar to AuthnRequest or should the library return an interface

I'm sending interface{} right now from the library, please suggest what should be the right way of doing this. I would prefer creating another struct in another package called DTO (or something similar) and embedding that struct into AuthnRequest in internal package and returning the dto from ValidateAuthnRequest method.

I hope I'm going in right direction with respect to the problem we are trying to solve.

[mayankagwl]

@avinashdhinwa Yes As we are not exposing the AuthnRequest structure to the public, you can create another struct with AuthRequest Property like (ID, IsPassive, ForceNAuth, etc) in parse.go file Because of function related to the Parsing the AuthnRequest. we can do it in the parse.go file

[avinashdhinwa]

@mayankagwl I believe it shouldn't be in parse.go as the methods' responsibility in parse.go is just to parse it, which it is already doing. What is we need is just a way to send data from ValidateAuthnRequest method, which can achieved either using a map in this method or creating a struct just for response. The benefit of having a struct is that so it's strongly typed. Also, I believe we can move parse.go in internal as well.

[mayankagwl]

@avinashdhinwa yes we can move parse.go under the internal package as well And for returning the Parsed value I also prefer the Struct pointer instead of map Thanks

[avinashdhinwa]

@mayankagwl Ahh.. I missed that a method in parse.go depended on IDP, so can't move it internal. I've created a struct, it has only four basic properties as of now. Please let me know if anything else needs to changed or added, moving PR from draft to ready for review.

[mayankagwl]

@avinashdhinwa And LoginRadius is also giving swags to people who will be contributing, so please make sure to read our blog and submit your info to get free goodies.

https://www.loginradius.com/engineering/page/hacktoberfest2020

[avinashdhinwa]

@mayankagwl Thanks for the support, it was the second PR which I've ever raised on an open source project and the first one to be merged.