socketry / async-http

MIT License
298 stars 45 forks source link

Support Basic Auth? #17

Closed ainar-g closed 5 years ago

ainar-g commented 5 years ago

While completely insecure, there are still legacy use cases for it. The stdlib has it, but async-http doesn't.

ioquatix commented 5 years ago

I haven't implemented it. How does it work?

ioquatix commented 5 years ago

@ainar-g I've been working pretty hard on this eco-system for the last few months. I need to take some time out for other projects.

If this is something really useful for you, do you think you can make a PR? I'd welcome it.

ainar-g commented 5 years ago

I appreciate the work. I will try to send a PR either this week or in the following weeks.

ioquatix commented 5 years ago

Even if it's just a POC hack, I am happy to take a look.

julik commented 5 years ago

There is no built-in support for it but you can implement it in the calling code pretty easily:

    authorization_encodded = [user , ':', password].join
    headers = [
      ['Authorization', 'Basic %s' % Base64.strict_encode64(authorization_encodded)]
    ]
    response = client.post(uri, headers, [request_body_string])
ioquatix commented 5 years ago

What would make the most sense is to add a basic auth header to http-protocol. Ideally, you'd write something like this:

headers = {
    authorization: HTTP::Protocol::BasicAuthorization.new(user, password)
}

I think it's better and simpler than trying to introduce a new API to take care of it, and as you said, it's not used that much any more.

svkampen commented 5 years ago

While completely insecure, there are still legacy use cases for it. The stdlib has it, but async-http doesn't.

It's not entirely insecure/legacy - Basic Auth over HTTPS is perfectly secure and still used in many cases (for instance, I'm currently working on something that uses the reddit API, whose initial authentication step uses Basic Auth before giving out a token). It would be nice to have in the library, but simply generating the header myself is working fine in the meantime.

ioquatix commented 5 years ago

Yeah, so I agree with all the different positions people are taking here. As I said, this is not a concurrency-related issue but a protocol one, so the issue belongs in the http-protocol gem.

ioquatix commented 4 years ago

It was implemented in Protocol::HTTP::Header::Authorization.

ioquatix commented 4 years ago

The interface for this changed in the latest version.

It's now

headers.add('authorization', Protocol::HTTP::Header::Authorization.basic(username, password))