Conversation
715fadc to
4f56ff1
Compare
|
This works now, tested against |
|
Just a note: In order to test this, you need to stand up some infrastructure for validating your WebBotAuth keys. Here is a Python script that will generate your private key and JWKS JSON file. This file needs to be served from your website at at An easy way to test is to hit |
| transfer._redirecting = false; | ||
|
|
||
| if (status == 401 or status == 407) { | ||
| if ((status == 401 or status == 407) and transfer.client.use_proxy) { |
There was a problem hiding this comment.
Is this added condition correct? I think it breaks authentication interception?
There was a problem hiding this comment.
It fixed an issue I was running into (where we would misinterpret the 401 returned by the crawltest page). I can recheck this, do we have any specific authentication interception tests to run against?
There was a problem hiding this comment.
There are some auth tests in demo, but I don't know if they test this specific path, @krichprollsch ?
src/browser/WebBotAuth.zig
Outdated
| for (b64) |ch| { | ||
| if (ch != '\n' and ch != '\r') { | ||
| clean[clean_len] = ch; | ||
| clean_len += 1; |
There was a problem hiding this comment.
Are we sure that it can never be > 4096?
src/browser/WebBotAuth.zig
Outdated
| } | ||
|
|
||
| var der: [128]u8 = undefined; | ||
| const decoded_len = try std.base64.standard.Decoder.calcSizeForSlice(clean[0..clean_len]); |
There was a problem hiding this comment.
Can't decoded_len > 128?
| app.robots = RobotStore.init(allocator); | ||
|
|
||
| app.http = try Http.init(allocator, &app.robots, config); | ||
| if (config.webBotAuth()) |wba_cfg| { |
There was a problem hiding this comment.
Is there a reason for the app to own this and not the Http? (I think I have the same question about robots).
There was a problem hiding this comment.
Put WebBotAuth in App just because that's where Robots was. Robots being in the App was something that Pierre preferred from what I remember. @krichprollsch
| return true; | ||
| } | ||
|
|
||
| if (std.mem.eql(u8, "--web_bot_auth_domain", opt)) { |
There was a problem hiding this comment.
Maybe these have well know meaning to users, but should we validate these? They're used very specifically, like this shouldn't include the protocol and shouldn't include the trailing slash. I realize that's what a "domain" is..but..
Also, keyid has to be base64 encoded. WE'll generate invalid JSON if this isn't well formed. Will that be an issue? Or will things just fail gracefully?
ea056be to
7c2df23
Compare
|
PR has gotten messed up...I think I've seen this before and it's just a github issue? But it makes it hard to review :/ |
|
The issue seems to be fixed? I just rebased again against |
|
I deployed a validation server on a temp URL for now: https://lightpanda-wbauth.fly.dev/.well-known/http-message-signatures-directory
Here is the private repository: https://github.com/lightpanda-io/wbauth |
This adds support for validating as a Web Bot using the Web Bot Auth protocol.