search

greenpau / caddy-security

🔐 Authentication, Authorization, and Accounting (AAA) App and Plugin for Caddy v2. 💎 Implements Form-Based, Basic, Local, LDAP, OpenID Connect, OAuth 2.0 (Github, Google, Facebook, Okta, etc.), SAML Authentication. MFA/2FA with App Authenticators and Yubico. 💎 Authorization with JWT/PASETO tokens. 🔐
https://authcrunch.com/
Apache License 2.0
1.49k stars 73 forks source link

Fix: Custom HTML Headers and static assets got lost on caddy adapt #290

Open poettig opened 1 year ago

poettig commented 1 year ago

When running caddy adapt, the config of an app gets marshaled into JSON. Because custom html header path and static_asset were not part of the config portal.UI, they did not get marshaled, leading to these settings to be lost when using caddy adapt and then running caddy with the resulting JSON instead of the Caddyfile.

This merge request fixes that problem by moving the actual loading of the assets to go-authcrunch and only saving the values into the configuration when read from a Caddyfile.

I needed this because I run https://github.com/mholt/caddy-l4, which does not work with Caddyfiles.

poettig commented 1 year ago

Dependant pull request in go-authcrunch: https://github.com/greenpau/go-authcrunch/pull/49

poettig commented 1 year ago

(Kind of) minimal Caddyfile for reproduction:

{
    debug
    order authenticate before respond
    security {
        oauth identity provider google {
            realm google
            driver google
            client_id CLIENT_ID
            client_secret CLIENT_SECRET 
            scopes openid email
        }

        authentication portal testportal {
            crypto key sign-verify JWT_TOKEN
            enable identity provider google

            ui {
                logo url /assets/test.png
                static_asset "assets/test.png" "image/png" /tmp/test.png
                custom html header path /tmp/head.html
            }
        }
    }
}

http://localhost:4000 {
    authenticate with testportal
}
greenpau commented 11 months ago

@poettig , please add CLA to assets/cla/consent.yaml

github-actions[bot] commented 11 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

poettig commented 11 months ago

I have read the CLA Document and I hereby sign the CLA

poettig commented 11 months ago

@greenpau I rebased against upstream and signed using the CLA workflow. Should be cleared up now.

greenpau commented 11 months ago

@poettig , please add your consent to assets/cla/consent.yaml.

greenpau commented 11 months ago

@poettig , please rebase to pick up go-authcrunch v1.0.46

greenpau commented 11 months ago

@poettig . please create PR here and describe how this feature works. https://github.com/authp/authp.github.io

greenpau commented 11 months ago

Once I review the docs, I will merge this one.

poettig commented 11 months ago

@greenpau I don't think there is anything to add to the documentation. I just made Custom HTML Template Header and Static Assets work when running caddy adapt to convert a Caddyfile into a Caddy JSON configuration file. Before this PR, it only works with a Caddyfile or when writing the Caddy JSON config manually, because these two configuration options were lost when running caddy adapt.

I might have understood something wrong about your request, so I'm open for clarifications :)

greenpau commented 10 months ago

@poettig , please help me understand this.

You removed the following. why? (note that I already don't remember what it does 😄)

            for k, v := range ui.PageTemplates {
                    headIndex := strings.Index(v, "<meta name=\"description\"")
                    if headIndex < 1 {
                        continue
                    }
                    v = v[:headIndex] + string(b) + v[headIndex:]
                    ui.PageTemplates[k] = v
                }
poettig commented 10 months ago

@greenpau As I understood it, this reads the specified file with custom HTML tags for inside the <head> tag and inserts them after the <meta name="description"> tag.

This behaviour was different to the other cases, which only store the config option into the UI struct (which in turn gets marshalled into JSON by caddy if requested). Therefore, I equalized the behaviour by moving the code to https://github.com/greenpau/go-authcrunch/blob/main/pkg/authn/portal.go#L492

The code is not deleted, just moved to somewhere else :)