amdonov / lite-idp

Lightweight SAML Identity Provider
Apache License 2.0
210 stars 48 forks source link

Configurable UI handler #9

Closed dmorgan81 closed 5 years ago

dmorgan81 commented 6 years ago

Like with the other handlers on the IDP type, allow clients to specify their own UI handler. By default use the bundled UI from the hack directory.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 28


Totals Coverage Status
Change from base Build 26: 0.04%
Covered Lines: 1052
Relevant Lines: 1456

💛 - Coveralls
amdonov commented 5 years ago

HandlerFuncs are used for all of the other paths because they are methods on the IdP struct. I'm inclined to leave the UI as a Handler rather than change it to a HandlerFunc. If you make that change, I'll approve the request. Also, I'm not sure the change is required to get access to the behavior you're trying to modify. The final handler used by the IdP is public and can be modified using standard middleware patterns. For example, could you do something like the following? That's how logging and HSTS are added to the IdP in the default serve command.

    i := &IDP{}
    h, err := i.Handler()
    if err != nil {
        panic(err)
    }
    myHandler := func() http.Handler {
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            // Intercept and modify UI requests here
            h.ServeHTTP(w, r)
        })
    }
    http.ListenAndServeTLS("0.0.0.0:8443", "", "", myHandler())
dmorgan81 commented 5 years ago

I changed it to be just a http.Handler instead of a HandlerFunc.

Your alternative approach would certainly work, but it means ServeCmd is not reusable in scenarios where downstream needs to change the UI.