Kitura / Kitura-CredentialsHTTP

A plugin for the Kitura-Credentials framework that authenticates using HTTP Basic and Digest authentication
Apache License 2.0
16 stars 14 forks source link

HTTP Basic Auth is providing unauthorized access to protected resources. #59

Closed dillan closed 5 years ago

dillan commented 5 years ago

I'm working on setting up HTTP Basic Auth and I think I may have found a nasty bug. When I go to the OpenAPI UI and access one of the protected routes, I get the username and password prompt as expected. When I enter an incorrect username and password, the prompt reappears so I can try again with different credentials (expected). The issue is that if I hit the cancel button on the prompt, the server returns a 401 HTTP Status code (expected), but also returns the data that they are not authorized to access (unexpected).

Here is how I setup the Basic Auth:

func initializeBasicAuth(app: App) {
    let credentials = Credentials()

    let basicCredentials = CredentialsHTTPBasic(verifyPassword: { username, password, callback in
        UserAuth.authEntry(username: username, onCompletion: { (authMatches, error) in
            if let entries = authMatches, let userAuth = entries.first {

                // Check Password
                let saltedPassword = "\(password)\(userAuth.salt)"
                let hashedPassword = hashPassword(saltedPassword)

                if hashedPassword == userAuth.hashedPassword {
                    let userProfile = UserProfile(id: userAuth.username,
                                                  displayName: username,
                                                  provider: "HTTPBasic")
                    callback(userProfile)
                } else {
                    callback(nil)
                }
            } else {
                callback(nil)
            }
        })
    })

    credentials.register(plugin: basicCredentials)
    app.router.get("/entries", middleware: credentials)
    app.router.put("/entries", middleware: credentials)
    app.router.delete("/entries", middleware: credentials)
}

So if I use the OpenAPI UI to perform a GET request on the /entries endpoint, I still get the JSON data back in the body of the response along with the 401 HTTP Status code in the headers.

dillan commented 5 years ago

Also confirmed by Pushkar Kulkarni. From Slack conversation:

https://swift-at-ibm.slack.com/archives/C29BLC4KB/p1560408275024600?thread_ts=1560376184.023800&cid=C29BLC4KB

pushkarnk Yup, this is easily reproducible. And it happens in a browser window too. So, doesn't look like an OpenAPI UI issue alone. It appears like HTTPBasic auth is broken

dillan commented 5 years ago

It may prove a good idea to add a unit test for this as well.

pushkarnk commented 5 years ago

It turned out that the Credentials middleware was configured after configuring the route handler.

IMO, this critical nature of the order in which middleware is configured may not be clear to new users. As @dillan asked over slack, I'm not sure if it is possible to have any safeguards against doing these in a wrong order. Any thoughts @djones6 / @ianpartridge ?

The Routing documentation does talk about an order of execution for middleware and route handlers. But may be we should document this user error as a warning there? cc @helenmasters

helenmasters commented 5 years ago

@pushkarnk thanks for the heads up. @Andrew-Lees11 is going to add a line into the Routing documentation on kitura.io about the order of execution and we're also going to start work on a middleware guide, which should go into more detail about the order of execution.

pushkarnk commented 5 years ago

Thanks @helenmasters