I have come up with the use case where I feel we need to make a small change to the way strict keys are coded. Basically I'm setting up a webservice which stores data based on a by client location. Given what we have at the moment the domain has be passed in as a seperate parameter. This provides a security hole as I can pass in my valid hash and then map to a different domain.

The patch attatch allows the key to be accessed as an argument in the under lying webservice call but doesn't change the signature for the webservices module. That way all routing etc is based on the value.

CommentFileSizeAuthor
#4 apikeys.patch965 bytesmarcingy
api_key.patch487 bytesmarcingy

Comments

marcingy’s picture

Assigned: Unassigned » marcingy
marcingy’s picture

Status: Needs review » Needs work

This patch needs some work as optional parameters will be affected by this if they aren't present as it is just appending to the end of the array.

marcingy’s picture

Two options exist as I see it

1. Create a session variable holding the api
2. Create a function which stores a key in static variable and allows services to retrieve its value.

On further review SESSION variables are created in core. I'll take this approach for now and can revisit in the future.

New patch will be rolled in the next couple of days

marcingy’s picture

Status: Needs work » Needs review
StatusFileSize
new965 bytes

patch using a static variable.

marcingy’s picture

Can someone review and commit please

magico’s picture

I think we need to change the way key values are passed.

Instead of:
* having keys enable/disable which break any client developed for the service; and,
* passing 4 parameters when a function uses keys

We should always pass one argument that:
* when keys are enabled in the server, that argument is validated against a authorization scheme
* if keys are disabled the argument is ignored by the server (despite of any that it has)
* the clients are never broken by this
* we have enough flexibility to develop different authorization hash methods

marcingy’s picture

Status: Needs review » Closed (fixed)