Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Basic Auth for a server #64

Closed
bdw429s opened this issue Jan 25, 2017 · 10 comments
Closed

Support Basic Auth for a server #64

bdw429s opened this issue Jan 25, 2017 · 10 comments
Milestone

Comments

@bdw429s
Copy link
Member

bdw429s commented Jan 25, 2017

https://github.com/undertow-io/undertow/blob/master/examples/src/main/java/io/undertow/examples/security/basic/BasicAuthServer.java

If we can pass a list of usernames and passwords via a new server option to Runwar then I can wrap this up in CommandBox.

@yukido
Copy link

yukido commented Feb 8, 2017

I strongly support this.

@denuno denuno added this to the 3.6.0 milestone Mar 5, 2017
@denuno
Copy link
Member

denuno commented Mar 5, 2017

So what I went with here is an option called -users or --basicauthusers and it expects the format to be like this: bob=secret,alice=fun,equals=blah\=inpass (escape = with \=)

--basicauthusers="bob=wee,al=hoo"

There's also --basicauth-enable which you can use to turn it off even if users are specified. (Specifying users automatically turns it on otherwise.)

@bdw429s
Copy link
Member Author

bdw429s commented Mar 6, 2017

Excellent. Here is the CommandBox ticket for it to be incorporated
https://ortussolutions.atlassian.net/browse/COMMANDBOX-560

@denuno denuno closed this as completed in 958867c Mar 10, 2017
@bdw429s
Copy link
Member Author

bdw429s commented Apr 21, 2017

@denuno How should commas be escaped (likely in passwords that uses punctuation)? What do you think about using a scheme like URL encoding since there's already libs/built-ins out there for encoding/decoding for it? This would also take care of things like a double quote in a password.

--basicauthusers="bob=secret&alice=fun&equals=blah%3Dinpass&punctuation=%23%24%25%5E%26*(%2C.%2F%3C%3E%3F"

@bdw429s
Copy link
Member Author

bdw429s commented Apr 21, 2017

Looking at your code it seems you already accounted for commas to be passed as \, so that's cool. We can roll with this for now.

@bdw429s
Copy link
Member Author

bdw429s commented Apr 21, 2017

@denuno Works pretty good. Before we set this in stone, what do you think about the ability to apply basic auth to a specific sub folder or path?

  1. Does Undertow support that?
  2. How would the config deal with it?
  3. Is it worth it?

@denuno
Copy link
Member

denuno commented Apr 21, 2017

For 1, we can use a PathHandler which would limit it to a path or paths, so that infrastructure exists.
For 2 & 3... well, if we could come up with something good for 2, it makes 3 more justified.

I rather like the idea of being able to plug in some auth easily. Configuration is an issue though, especially with limits on how long command lines can be and whatnot, and with various auth schema needing various types of info. I'm kinda thinking like, instead of passing various users/groups/paths/whatnot, we instead pass in a file path to a JSON file containing that info.

I'm often using various 3rd party auth providers, it would be kinda cool to have container-level support for requiring auth based on the path... I think there's even a jee spec for doing so, ish, which might give some inspiration as to some type of generic user/group/rule define'n possibilities.

@bdw429s
Copy link
Member Author

bdw429s commented Apr 21, 2017

From the CommandBox side of things, the setup looks like this:

{
    "web":{
        "basicAuth":{
            "users":{
                "brad":"wood",
                "lu,is":"maj=ano"
            },
            "enable":"true"
        }
    }
}

A suggestion for #2 could look like this which would leave the default whole-site protection simple.

{
    "web":{
        "basicAuth":{
            "users":{
                "brad":"wood",
            },
            paths : {
                "/foo" : {
                   "users":{
                        "foouser":"foopass",
                    }
                },
                "/admin/bar" : {
                   "users":{
                        "adminuser":"adminass",
                    }
                }
            }
            "enable":"true"
        }
    }
}

Or here's a simpler version that assumes there's nothing other than users to configure for a path:

{
    "web":{
        "basicAuth":{
            "users":{
                "brad":"wood",
            },
            paths : {
                "/foo" : {
                     "foouser":"foopass",
                },
                "/admin/bar" : {
                   "adminuser":"adminass",
               }
            }
            "enable":"true"
        }
    }
}

Of course, this is all on the CommandBox side of things. I've still got to wrap it all up and send it over to Runwar.

@bdw429s
Copy link
Member Author

bdw429s commented May 19, 2017

@denuno Someone pointed out to me today that the basic auth doesn't quite seem to be 100% yet. Once you log in, they expected the cgi.remote_user variable to have the username, but it's blank in instead. I assume some extra plumbing just needs put in to set that?

https://ortussolutions.atlassian.net/browse/COMMANDBOX-625

@bdw429s
Copy link
Member Author

bdw429s commented May 19, 2017

Looking at Undertow, I assume we need to be using this class?
https://github.com/undertow-io/undertow/blob/master/core/src/main/java/io/undertow/attribute/RemoteUserAttribute.java

denuno added a commit that referenced this issue Sep 27, 2017
* Replace the default/catchall log handler with a name.contains( 'cfml' || 'undertow' ) filtered one.

* Override web.xml REST mappings when passed in as argument. Refs #46

* Bump version to 3.6.0 and update Undertow. Closes #62

* Perhaps fix chrome as an option for browser opening, also first try preferred browser vs. firefox.

* Fix relative resource path stuff, and greedy alias replacement. Closes #63

* Tone down the debug stuff.  Refs #56

* try/catch (#68)

* Try to fix some NPEs from the filters

* Update Undertow to 1.4.11.Final. Closes #62 (at least until the next version comes out.)

* Improve SSL support and add a Basic Auth feature.  Might close #69 and close #64

* Add some performance tuning options for Undertow.  Closes #71

* Allow aliases to be case insensitive.  Closes #52

* Initial go at some load balancer / clustering stuff.  Refs #73

* Add JSR websocket support.  Closes #53

* backing up for a second to cut 3.6.0

* Release 3.6.0

* Bump to 3.6.1, put the loadbalance stuff back in.

* Fix wildcard cert loading and add rough draft of adding/removing nodes from the balancer.

* Add --directory-refresh option, defaulted to false. Closes #75 (until something better comes along)

* Update loadbalancer stuff a bit, add caching resource manager for testing

* Little better error when balancehost is incomplete.

* Fix case sensitivy (#77)

Thanks!

* Improve ACF detection & fix missing cfcalsses folder (#78)

* Add option to turn on proxy peer address handling (-proxypeeraddress,--proxy-peeraddress <true|false>).  Closes #80

* Add option to disable system tray (-tray, --tray-enable <true|false>).  Closes #81

* Add missing URLRewriteFilter init parameters (--urlrewrite-check <interval>, --urlrewrite-statuspath <path>).  Closes #79

* Add support for HTTP2.  Closes #72

* Update tray dependencies. Refs #82

* Add Tray class

* Add NullPrintStream class

* bump jre

* Fix weird web.xml parse error, and add tray action to open file system browser.

* Shutdown enhancements.

* Looking at wrong flag (#84)

Thanks!

* Logging enhacements (#87)

* Ensure we pass SystemTray a tooltip < 65 chars long.

* Remove logback, add slf4j.

* Looking at wrong flag (#84)

Thanks!

* Logging enhacements (#87)

* Remove logback, add slf4j.

* Suppress some startup stuff, both from slf4j and the availability check.

* Default open-browser to true if url is specified.

* A test for debugyness

* Move try/catch to to ServerOptions versus everywhere else.

* Add PID and other Tray things.  Closes #89, Closes #90

* Tweak restart, don't think it's going to work though.

* Rework the readme (#94)

* Update the README

* Update auth so remote_user is set. Closes #83

* Remove unused imports

* Bump version for next release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants