golangci: require comments, add a few missing ones

.golangci.yml

@@ -71,12 +71,8 @@ linters-settings:
       - unreachable
       - unsafeptr
       - unusedresult
+issues:
+  exclude-use-default: false
+  exclude:
+    - G104 # 'Errors unhandled. (gosec)
 
-#issues:
-#  exclude-use-default: false
-#  exclude:
-#    - G104 # 'Errors unhandled. (gosec)
-#    - G204 # Subprocess launched with variable (gosec)
-#    - G301 # Expect directory permissions to be 0750 or less (gosec)
-#    - G302 # Expect file permissions to be 0600 or less (gosec)
-#    - G304 # Potential file inclusion via variable (gosec)

identity_provider.go

@@ -372,7 +372,7 @@ func (req *IdpAuthnRequest) Validate() error {
 	// For now we do the safe thing and fail in the case where we think
 	// requests might be signed.
 	if idpSsoDescriptor.WantAuthnRequestsSigned != nil && *idpSsoDescriptor.WantAuthnRequestsSigned {
-		return fmt.Errorf("Authn request signature checking is not currently supported")
+		return fmt.Errorf("authn request signature checking is not currently supported")
 	}
 
 	// In http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf §3.4.5.2

samlsp/middleware.go

@@ -130,6 +130,8 @@ func (m *Middleware) RequireAccount(handler http.Handler) http.Handler {
 	return http.HandlerFunc(fn)
 }
 
+// RequireAccountHandler handles an HTTP request that does not already have a
+// valid session. It redirects the user to start the SAML auth flow.
 func (m *Middleware) RequireAccountHandler(w http.ResponseWriter, r *http.Request) {
 	// If we try to redirect when the original request is the ACS URL we'll
 	// end up in a loop. This is a programming error, so we panic here. In

schema.go

@@ -39,6 +39,10 @@ type AuthnRequest struct {
 	ProviderName                   string `xml:",attr"`
 }
 
+// LogoutRequest  represents the SAML object of the same name, a request from an IDP
+// to destroy a user's session.
+//
+// See http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf
 type LogoutRequest struct {
 	XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol LogoutRequest"`
 

service_provider.go

@@ -452,16 +452,7 @@ func (sp *ServiceProvider) validateDestination(response []byte, responseDom *Res
 }
 
 // ParseResponse extracts the SAML IDP response received in req, validates
-// it, and returns the verified attributes of the request.
-//
-// This function handles decrypting the message, verifying the digital
-// signature on the assertion, and verifying that the specified conditions
-// and properties are met.
-//
-// If the function fails it will return an InvalidResponseError whose
-// properties are useful in describing which part of the parsing process
-// failed. However, to discourage inadvertent disclosure the diagnostic
-// information, the Error() method returns a static string.
+// it, and returns the verified assertion.
 func (sp *ServiceProvider) ParseResponse(req *http.Request, possibleRequestIDs []string) (*Assertion, error) {
 	now := TimeNow()
 	retErr := &InvalidResponseError{
@@ -484,6 +475,17 @@ func (sp *ServiceProvider) ParseResponse(req *http.Request, possibleRequestIDs [
 
 }
 
+// ParseXMLResponse validates the SAML IDP response and
+// returns the verified assertion.
+//
+// This function handles decrypting the message, verifying the digital
+// signature on the assertion, and verifying that the specified conditions
+// and properties are met.
+//
+// If the function fails it will return an InvalidResponseError whose
+// properties are useful in describing which part of the parsing process
+// failed. However, to discourage inadvertent disclosure the diagnostic
+// information, the Error() method returns a static string.
 func (sp *ServiceProvider) ParseXMLResponse(decodedResponseXML []byte, possibleRequestIDs []string) (*Assertion, error) {
 	now := TimeNow()
 	var err error
@@ -520,11 +522,11 @@ func (sp *ServiceProvider) ParseXMLResponse(decodedResponseXML []byte, possibleR
 	}
 
 	if resp.IssueInstant.Add(MaxIssueDelay).Before(now) {
-		retErr.PrivateErr = fmt.Errorf("IssueInstant expired at %s", resp.IssueInstant.Add(MaxIssueDelay))
+		retErr.PrivateErr = fmt.Errorf("response IssueInstant expired at %s", resp.IssueInstant.Add(MaxIssueDelay))
 		return nil, retErr
 	}
 	if resp.Issuer.Value != sp.IDPMetadata.EntityID {
-		retErr.PrivateErr = fmt.Errorf("Issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID)
+		retErr.PrivateErr = fmt.Errorf("response Issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID)
 		return nil, retErr
 	}
 	if resp.Status.StatusCode.Value != StatusSuccess {
@@ -627,20 +629,20 @@ func (sp *ServiceProvider) validateAssertion(assertion *Assertion, possibleReque
 			}
 		}
 		if !requestIDvalid {
-			return fmt.Errorf("SubjectConfirmation one of the possible request IDs (%v)", possibleRequestIDs)
+			return fmt.Errorf("assertion SubjectConfirmation one of the possible request IDs (%v)", possibleRequestIDs)
 		}
 		if subjectConfirmation.SubjectConfirmationData.Recipient != sp.AcsURL.String() {
-			return fmt.Errorf("SubjectConfirmation Recipient is not %s", sp.AcsURL.String())
+			return fmt.Errorf("assertion SubjectConfirmation Recipient is not %s", sp.AcsURL.String())
 		}
 		if subjectConfirmation.SubjectConfirmationData.NotOnOrAfter.Add(MaxClockSkew).Before(now) {
-			return fmt.Errorf("SubjectConfirmationData is expired")
+			return fmt.Errorf("assertion SubjectConfirmationData is expired")
 		}
 	}
 	if assertion.Conditions.NotBefore.Add(-MaxClockSkew).After(now) {
-		return fmt.Errorf("Conditions is not yet valid")
+		return fmt.Errorf("assertion Conditions is not yet valid")
 	}
 	if assertion.Conditions.NotOnOrAfter.Add(MaxClockSkew).Before(now) {
-		return fmt.Errorf("Conditions is expired")
+		return fmt.Errorf("assertion Conditions is expired")
 	}
 
 	audienceRestrictionsValid := len(assertion.Conditions.AudienceRestrictions) == 0
@@ -650,7 +652,7 @@ func (sp *ServiceProvider) validateAssertion(assertion *Assertion, possibleReque
 		}
 	}
 	if !audienceRestrictionsValid {
-		return fmt.Errorf("Conditions AudienceRestriction does not contain %q", sp.MetadataURL.String())
+		return fmt.Errorf("assertion Conditions AudienceRestriction does not contain %q", sp.MetadataURL.String())
 	}
 	return nil
 }