Building requests correctly

[oneiros]

One thing that bugged me from the beginning was the reliance on Rack::Request. I touched a bit on that in #6, but the gist of it is that

1. You have this weird chicken-and-egg problem, where you need to first create a request to build the signature, that you then put into another request that you then actually perform. But more importantly:
2. You cannot actually make outgoing requests with rack, so when you only want to make requests, why should you have to deal with rack?

But I thought it kind of works, so I guess it is OK. Until I encountered a problem:

The second parameter of Linzer.new_request is called uri, but in fact it MUST be a path. You can pass a full URI, but then all kinds of weird things can happen. This parameter is passed as PATH_INFO into rack's environment, so it absolutely should never contain anything but the path according to https://github.com/rack/rack/blob/main/SPEC.rdoc

So how do you create a request that includes everything you might want to sign or verify? It is possible, but not straight-forward at all. I guess you would need to pass at least the Host header, something like rack.url_scheme and possibly QUERY_STRING if one is part of the URI.

Having all this information is especially important if you want to sign or verify the @target-uri derived component or query parameters. This also touches on #9.

I do not think users of linzer should have to worry about this. This is cumbersome, redundant and error-prone.

I am, however, not sure how to fix this. I see three different options:

1. Quick fix

Linzer::Request.build could parse the passed uri parameter, e.g. with URI.parse and build a more full-featured rack environment from that.

2. Small Refactor

Linzer::Request could become a very simple class, that gets passed method, URI and headers and offers simple accessor methods to query the components that Linzer::Message needs to know. Kind of a smaller, simpler version of Rack::Request that encapsulates only what is needed to create or verify signatures.

This could offer a class method, e.g. ::from_rack, that accepts a Rack::Request and extracts the necessary information from there, making it reasonably easy to use e.g. in the included rack middleware.

This approach would make it straight-forward to construct a request object that includes all information necessary for signing and verification. It would also mean you could sign requests without using rack at all.

3. Large Refactor

This is the idea I outlined in #6: linzer could have a Linzer::Request base class that defines the necessary accessor methods and a couple of adapter classes that inherit from that and encapsulate a request object from a different library. To begin with there could be adapters for Rack::Request and Net::HTTPRequest, but this should be easy to extend so everyone can create adapters for their preferred HTTP library.

This would, among other things, unlock the one-step signing proposed in #6.

(Note that I only mentioned requests here for brevity, but of course the same applies to responses as well.)

I would be happy to work on this, just let me know which direction you would prefer or if you have any other ideas.

[nomadium]

One thing that bugged me from the beginning was the reliance on Rack::Request. I touched a bit on that in #6, but the gist of it is that

Me too, this was clearly a hack to iterate quickly at the beginning and I have taken too long to clean it up. I reached out for Rack::Request as a mean to have a standardized and rich ruby class representing a HTTP request but it was always weird to rely on Rack for use cases where a user just want to send a signed POST request and probably doesn't have context or care about what is Rack in the first place. I should have relied on Net::HTTPRequest or wrap it from the beginning, allowing for adapters to accommodate for other ruby libraries/frameworks like Rack.

[...]

I do not think users of linzer should have to worry about this. This is cumbersome, redundant and error-prone.

100% agree.

I am, however, not sure how to fix this. I see three different options:

• 1. Quick fix
• 2. Small Refactor
• 3. Large Refactor

I would be happy to work on this, just let me know which direction you would prefer or if you have any other ideas.

What I'd like to do finally address this is:

• Address the issue mentioned in the option 1 (quick fix).
• Give me a couple of days or so to get back to you on how to proceed with the proposed refactor on option 3. I'd like to take a stab at it myself first and review it with you if you don't mind. If for any reason, this takes me too long and you are blocked because of this, I'd advise you to take the lead, drive the refactor and I can review it, of course.

Let me get back to you on this soon.

[oneiros]

Give me a couple of days or so to get back to you on how to proceed with the proposed refactor on option 3. I'd like to take a stab at it myself first and review it with you if you don't mind.

Of course, that would be totally fine! Thanks for looking into this.

[nomadium]

Give me a couple of days or so to get back to you on how to proceed with the proposed refactor on option 3. I'd like to take a stab at it myself first and review it with you if you don't mind.

Of course, that would be totally fine! Thanks for looking into this.

Quick update on this one: I made progress with the refactor to address this issue on how requests and responses should be built correctly instead of relying on Rack unnecessarily. These changes also unblock the implementation of single step signing and verification as suggested in #6.

I'm satisfied with the results so far and I'll try to publish a PR for review later today, I still have to clean-up a couple of things to make my branch ready for review and merge. 😀

[nomadium]

Same as #6. 😀

Once you have a chance to test the new stable release, I'd appreciate feedback if you still consider this issue should remain open or can be closed, thanks!

[oneiros]

This is fixed and very thoroughly so. Thanks a lot!