Support SAML authentication

[6543]

Resubmit #25165 for review

Closes #5512

This PR adds basic SAML support

• Adds SAML 2.0 as an auth source
• Adds SAML configuration documentation
• Adds integration test:
  • Use bare-bones SAML IdP to test protocol flow and test account is linked successfully (only runs on Postgres by default)
  • Adds documentation for configuring and running SAML integration test locally

Future PRs:

• Support group mapping
• Support auto-registration (account linking)
• Convert helper classes in this PR to tailwind
• Test if github.com/crewjam/saml is more suited

Co-Authored-By: @jackHay22

[6543]

as per #29358 and #25165 (comment)

the pull is resubmitted for review

[Zettat123]

I found a more popular package(crewjam/saml) that we can use. It looks like russellhaering/gosaml2 only supports HTTP POST binding, but crewjam/saml supports both HTTP Redirect binding and HTTP POST binding.

[6543]

if you make a pull against my feature branch we can test that lib :)

[6543]

PS: it's no simple drop-in replacement :/

[6543]

@Zettat123 the pull is not originally from me but was just resubmited for review.

so @techknowlogick or @jackHay22 might know more about why thy have choosen the lib?

[6543]

@Zettat123 I'll added it to the followups is that ok or should we wait for the others to respond?

[jackHay22]

I don't have a strong opinion here; if the additional features are useful this would be a good time to make the switch.

[jackHay22]

@Zettat123 It looks like gosaml2 supports HTTP redirect binding. Let me know if I'm missing something.

[Zettat123]

Found an issue related to the Redirect binding. I haven't looked into this issue, but it looks like there is a workaround. But I think this issue not being solved for a long time means that the project is not very active.

[jackHay22]

I don't disagree but I think the crewjam/saml repo also has signs of inactivity.

[6543]

I think at some point a saml lib is just completed and only needs version bumps if it has dependencys?

so in this regards if there are no issues open anymore I'm fine with a inactive lib.

here both repos still have open issues :/

[grawity]

I don't think this is true? I've been using SimpleSAMLphp and it does accept SP certificates in the XML-format metadata directly.

[jackHay22]

That would make sense; I just wasn't able to get it to work. Perhaps the version we use doesn't support it?

[techknowlogick]

@grawity could you provide an example of your config for this so we can adjust the tests?

[grawity]

I wasn't able to find the sources of the Docker image being used, so I don't know what it does with the SP metadata, but given the separate settings to specify ACS/SLO endpoints, I suspect it doesn't look at the XML metadata at all and just builds its own version like this?

But on a full SimpleSAMLphp 2.x installation (and I believe 1.19.x as well), you can list the whole XML file in config.php and it will happily accept signed requests. (Although plain sign-on requests don't use the SP certificate at all; it's just a) encrypted assertions or b) signed single-logout requests that need it; but I re-tested the latter to verify that SimpleSAMLphp pays attention to the certificate in the SP's XML file and it does.)

$config = [
    ...
    "metadata.sources" => [
        ["type" => "flatfile"],                             // the PHP-format saml20-sp-remote.php
        ["type" => "xml", "file" => "metadata/gitea.xml"],  // an XML-format metadata file
    ],
];

Even if you're using the PHP-format SP metadata file (saml20-sp-remote.php), where it might look like the certificate has to be in a separate file:

$metadata["https://SOME_ENTITY_ID"] = [
    "AssertionConsumerService" => "https://SOME_URL",
    "certificate" => "exlibris-alma.crt",
    ...
];

it is still possible to specify it inline, and the XML-to-PHP metadata converter that comes with SimpleSAMLphp (Admin > Federation) will include any found certificates inline as part of the PHP-format metadata:

$metadata["https://SOME_ENTITY_ID"] = [
    "AssertionConsumerService" => "https://SOME_URL",
    "keys" => [
        ["type" => "X509Certificate", "encryption" => true, "signing" => true, "X509Certificate" => "MII…"]
    ]
    ...
];

[jackHay22]

Here's the repository with the configuration: https://github.com/AllSpiceIO/docker-test-saml-idp/tree/master

[jrjake]

I'm not certain on how thorough Gitea's requirements for test cases are, so take this with a grain of salt. But I don't think it's important to test SAML request signing because Gitea isn't making any claims that the IdP will rely on for the request. Also, (I sure hope) IdPs will be using HTTPS so there's no concern of tampering or confidentiality leaks anyways (not that there's any confidential data being sent by Gitea to begin with).

[6543]

We need to know soon, if switching the library is a breaking change !
cc @jackHay22

if it its, we should move it into the next release else merge it early and build upon it

[denyskon]

@techknowlogick @jackHay22 @Zettat123 We need a decision soon, otherwise this PR won't be part of Gitea 1.22

[silverwind]

Tailwind migration done.

[6543]

Thanks :)

[ldomesjo]

Will this make it into the 1.23 release? I've been looking forward for this feature :)

[techknowlogick]

Even though @6543 pinged me when we last saw each other, I'm sorry for letting this one drop off my radar. I think we should seriously consider switching to the other SAML lib as reading more into the spec it seems that the missing option from the existing library in use in this PR is a somewhat common option to use. I have access to push to this branch so I can resolve existing conflicts and start to work on that (I don't want to ask others to do the work, as they have already done so much, and to ask to do even more would be too much to ask).

[6543]

If you want you can also fetch this branch and create your own pull, as this would indicate you as the main author again 😅 ... anyway good to hear you had time to read the actual spec. can you suggest an library or is it to early? @techknowlogick