Skip to content
This repository has been archived by the owner on Oct 14, 2020. It is now read-only.

Add express/connect support #5

Closed
wants to merge 5 commits into from
Closed

Conversation

cmawhorter
Copy link

This mainly adds connect/express middleware support.

Also included:

-Tests
-Some minor bug fixes
-Introduce new validateHttpRequest which operates async. Original functionality moved to validateHttpRequestSync along with backwards compat.
-Other minor things (like commenting out unwritten nonce test which was being displayed as "1 test pending")

@cmawhorter
Copy link
Author

Oh yeah... to use:

var express = require('express'),
    ofuda = require('ofuda');

var app = express();

app.use(express.logger('dev'));
app.use(express.bodyParser());

function asyncFindUserByAccessKey(reqAccessKeyId, callback) {
    // Lookup user in db
    callback(user);
}

var ofudaOptions = {
    serviceLabel: 'HMAC',
    debug: true
};

app.use(ofuda.middleware(ofudaOptions, function(reqAccessKeyId, callback) {
    asyncFindUserByAccessKey(reqAccessKeyId, function(user) {
        callback({
                accessKeyId: user.accessKeyId
            , accessKeySecret: user.accessKeySecret
        });
    });
}));

app.put('/', function(req, res) {

});

return app;

@wolfeidau
Copy link
Owner

I just pushed up my changes which may conflict, I will try pulling your changes in and tell you how it goes.

Sorry for the delay.

@wolfeidau
Copy link
Owner

BTW thank you very much for taking the time to do these changes it is much appreciated.

@cmawhorter
Copy link
Author

np. Thanks for getting it started! FWIW, just fetched your changes and
tests still pass. =p

On Wed, Feb 27, 2013 at 5:56 PM, Mark Wolfe notifications@github.comwrote:

BTW thank you very much for taking the time to do these changes it is much
appreciated.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-14212247
.

@wolfeidau
Copy link
Owner

I had a read over these changes and they really don't match what I want to do with this module.

Some tips for future pull requests.

  • Don't leave commented code in a pull request it looks untidy and rushed.
  • Be careful when using === string and such like
  • Try and match the code style of the target repo
  • Break things into smaller pieces when pushing

There are some awesome ideas in this pull request but it seems a bit rushed, I need to reflect on your changes and probably split this module up a bit to make the connect middleware simpler.

Cheers

@wolfeidau wolfeidau closed this Mar 1, 2013
@cmawhorter
Copy link
Author

I am a little confused why the result is closing instead of working with me into making this something you find acceptable?

I feel like I have to defend my honor here.

*Don't leave commented code in a pull request it looks untidy and rushed.

What?

*Be careful when using === string and such like
*Try and match the code style of the target repo

I'll take this one and the previous one together.

I never strict compare strings. If I did it, it was only to match existing code. Furthermore, I'm pretty sure the line in you're referring to was already that way and I just added "toLowerCase" to squash a bug I found.

In fact, it's a pet peeve of mine when I see people strict compare strings. Suchas typeof blah === 'function', which exists in the code right now.

I even went as far as trying to match your if statements... is it "if () {" or "if (){", with or without space? I went with include the space.

*Break things into smaller pieces when pushing

5 separate commits?

connect middleware simpler.

Not sure how much simpler it could be. At any rate, I'll be splitting this into it's own thing. Good luck to you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants