From 89aa3595f537a69d1c5724bf6fa401f161267bb1 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 28 Feb 2014 13:19:52 -0800 Subject: [PATCH 1/9] Set the correct status code for User.login See https://github.com/strongloop/loopback/issues/118 --- lib/models/user.js | 5 ++++- test/user.test.js | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/models/user.js b/lib/models/user.js index f35053b3..049f1262 100644 --- a/lib/models/user.js +++ b/lib/models/user.js @@ -149,11 +149,14 @@ User.login = function (credentials, include, fn) { } else if(credentials.username) { query.username = credentials.username; } else { - return fn(new Error('must provide username or email')); + var err = new Error('username or email is required'); + err.statusCode = 400; + return fn(err); } this.findOne({where: query}, function(err, user) { var defaultError = new Error('login failed'); + defaultError.statusCode = 401; if(err) { debug('An error is reported from User.findOne: %j', err); diff --git a/test/user.test.js b/test/user.test.js index 81764783..7f5cedbb 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -9,6 +9,9 @@ var userMemory = loopback.createDataSource({ describe('User', function(){ var validCredentials = {email: 'foo@bar.com', password: 'bar'}; + var invalidCredentials = {email: 'foo1@bar.com', password: 'bar1'}; + var incompleteCredentials = {password: 'bar1'}; + beforeEach(function() { User = loopback.User.extend('user'); User.email = loopback.Email.extend('email'); @@ -135,6 +138,40 @@ describe('User', function(){ }); }); + it('Login a user over REST by providing invalid credentials', function(done) { + request(app) + .post('/users/login') + .expect('Content-Type', /json/) + .expect(401) + .send(invalidCredentials) + .end(function(err, res){ + done(); + }); + }); + + it('Login a user over REST by providing incomplete credentials', function(done) { + request(app) + .post('/users/login') + .expect('Content-Type', /json/) + .expect(400) + .send(incompleteCredentials) + .end(function(err, res){ + done(); + }); + }); + + it('Login a user over REST with the wrong Content-Type', function(done) { + request(app) + .post('/users/login') + .set('Content-Type', null) + .expect('Content-Type', /json/) + .expect(400) + .send(validCredentials) + .end(function(err, res){ + done(); + }); + }); + it('Returns current user when `include` is `USER`', function(done) { request(app) .post('/users/login?include=USER') From 57ff242308f7651e64ca8062b24d7d79e9d8f164 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Mon, 10 Mar 2014 18:05:44 -0700 Subject: [PATCH 2/9] Minor doc fix --- lib/loopback.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/loopback.js b/lib/loopback.js index e8b6c7af..6a057a44 100644 --- a/lib/loopback.js +++ b/lib/loopback.js @@ -57,7 +57,7 @@ loopback.mime = express.mime; */ loopback.compat = require('./compat'); -/** +/*! * Create an loopback application. * * @return {Function} @@ -305,7 +305,7 @@ loopback.autoAttachModel = function(ModelCtor) { } }; -/* +/*! * Built in models / services */ From 533daf4040d1f77a855b80468ecf041f9a5a0249 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 11 Mar 2014 22:37:54 -0700 Subject: [PATCH 3/9] Remove oauth2 models as they will be packaged in a separate module --- lib/models/oauth2.js | 222 ------------------------------------------- 1 file changed, 222 deletions(-) delete mode 100644 lib/models/oauth2.js diff --git a/lib/models/oauth2.js b/lib/models/oauth2.js deleted file mode 100644 index ba82a6e3..00000000 --- a/lib/models/oauth2.js +++ /dev/null @@ -1,222 +0,0 @@ -var loopback = require('../loopback'); - -// "OAuth token" -var OAuthToken = loopback.createModel({ - // "access token" - accessToken: { - type: String, - index: { - unique: true - } - }, // key, The string token - clientId: { - type: String, - index: true - }, // The client id - resourceOwner: { - type: String, - index: true - }, // The resource owner (user) id - realm: { - type: String, - index: true - }, // The resource owner realm - issuedAt: { - type: Date, - index: true - }, // The timestamp when the token is issued - expiresIn: Number, // Expiration time in seconds - expiredAt: { - type: Date, - index: { - expires: "1d" - } - }, // The timestamp when the token is expired - scopes: [ String ], // oAuth scopes - parameters: [ - { - name: String, - value: String - } - ], - - authorizationCode: { - type: String, - index: true - }, // The corresponding authorization code that is used to request the - // access token - refreshToken: { - type: String, - index: true - }, // The corresponding refresh token if issued - - tokenType: { - type: String, - enum: [ "Bearer", "MAC" ] - }, // The token type, such as Bearer: - // http://tools.ietf.org/html/draft-ietf-oauth-v2-bearer-16 - // or MAC: http://tools.ietf.org/html/draft-hammer-oauth-v2-mac-token-05 - authenticationScheme: String, // HTTP authenticationScheme - hash: String // The SHA-1 hash for -// client-secret/resource-owner-secret-key -}); - -// "OAuth authorization code" -var OAuthAuthorizationCode = loopback.createModel({ - code: { - type: String, - index: { - unique: true - } - }, // key // The string code - clientId: { - type: String, - index: true - }, // The client id - resourceOwner: { - type: String, - index: true - }, // The resource owner (user) id - realm: { - type: String, - index: true - }, // The resource owner realm - - issuedAt: { - type: Date, - index: true - }, // The timestamp when the token is issued - expiresIn: Number, // Expiration time in seconds - expiredAt: { - type: Date, - index: { - expires: "1d" - } - }, // The timestamp when the token is expired - - scopes: [ String ], // oAuth scopes - parameters: [ - { - name: String, - value: String - } - ], - - used: Boolean, // Is it ever used - redirectURI: String, // The redirectURI from the request, we need to - // check if it's identical to the one used for - // access token - hash: String // The SHA-1 hash for -// client-secret/resource-owner-secret-key -}); - -// "OAuth client registration record" -var ClientRegistration = loopback.createModel({ - id: { - type: String, - index: { - unique: true - } - }, - clientId: { - type: String, - index: { - unique: true - } - }, // key; // The client id - clientSecret: String, // The generated client secret - - defaultTokenType: String, - accessLevel: Number, // The access level to scopes, -1: disabled, 0: - // basic, 1..N - disabled: Boolean, - - name: { - type: String, - index: true - }, - email: String, - description: String, - url: String, - iconURL: String, - redirectURIs: [ String ], - type: { - type: String, - enum: [ "CONFIDENTIAL", "PUBLIC" ] - }, - - userId: { - type: String, - index: true - } // The registered developer - -}); - -// "OAuth permission" -var OAuthPermission = loopback.createModel({ - clientId: { - type: String, - index: true - }, // The client id - resourceOwner: { - type: String, - index: true - }, // The resource owner (user) id - realm: { - type: String, - index: true - }, // The resource owner realm - - issuedAt: { - type: Date, - index: true - }, // The timestamp when the permission is issued - expiresIn: Number, // Expiration time in seconds - expiredAt: { - type: Date, - index: { - expires: "1d" - } - }, // The timestamp when the permission is expired - - scopes: [ String ] -}); - -// "OAuth scope" -var OAuthScope = loopback.createModel({ - scope: { - type: String, - index: { - unique: true - } - }, // key; // The scope name - description: String, // Description of the scope - iconURL: String, // The icon to be displayed on the "Request Permission" - // dialog - expiresIn: Number, // The default maximum lifetime of access token that - // carries the scope - requiredAccessLevel: Number, // The minimum access level required - resourceOwnerAuthorizationRequired: Boolean -// The scope requires authorization from the resource owner -}); - -// "OAuth protected resource" -var OAuthResource = loopback.createModel({ - operations: [ - { - type: String, - enum: [ "ALL", "GET", "POST", "PUT", "DELETE", "HEAD", "OPTIONS", "PATCH" ] - } - ], // A list of operations, by default ALL - path: String, // The resource URI path - scopes: [ String ] -// Allowd scopes -}); - -// Use the schema to register a model -exports.OAuthToken = OAuthToken; -exports.OAuthAuthorizationCode = OAuthAuthorizationCode; -exports.ClientRegistration = ClientRegistration; -exports.OAuthPermission = OAuthPermission; -exports.OAuthScope = OAuthScope; -exports.OAuthResource = OAuthResource; From 9fb3f3313a2044e8e3d8a47cb1697b17489ba8b1 Mon Sep 17 00:00:00 2001 From: crandmck Date: Thu, 13 Mar 2014 17:25:21 -0700 Subject: [PATCH 4/9] Fixes to JSDoc for API docs --- lib/models/access-context.js | 8 ++++---- lib/models/application.js | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/models/access-context.js b/lib/models/access-context.js index fa3c8e72..9bbc1464 100644 --- a/lib/models/access-context.js +++ b/lib/models/access-context.js @@ -194,8 +194,8 @@ Principal.SCOPE = 'SCOPE'; /** * Compare if two principals are equal - * @param p The other principal - * @returns {boolean} + * Returns true if argument principal is equal to this principal. + * @param {Object} principal The other principal */ Principal.prototype.equals = function (p) { if (p instanceof Principal) { @@ -205,11 +205,11 @@ Principal.prototype.equals = function (p) { }; /** - * A request to access protected resources + * A request to access protected resources. * @param {String} model The model name * @param {String} property * @param {String} accessType The access type - * @param {String} permission The permission + * @param {String} permission The requested permission * @returns {AccessRequest} * @class */ diff --git a/lib/models/application.js b/lib/models/application.js index 2841a6e5..9af732f0 100644 --- a/lib/models/application.js +++ b/lib/models/application.js @@ -90,7 +90,7 @@ var ApplicationSchema = { modified: {type: Date, default: Date} }; -/** +/*! * Application management functions */ @@ -189,8 +189,7 @@ Application.resetKeys = function (appId, cb) { /** * Authenticate the application id and key. * - * `matched` will be one of - * + * `matched` parameter is one of: * - clientKey * - javaScriptKey * - restApiKey @@ -201,7 +200,7 @@ Application.resetKeys = function (appId, cb) { * @param {String} key * @callback {Function} callback * @param {Error} err - * @param {String} matched - The matching key + * @param {String} matched The matching key */ Application.authenticate = function (appId, key, cb) { this.findById(appId, function (err, app) { From c80b3341302cfca22079eaf066935f27adfe52e9 Mon Sep 17 00:00:00 2001 From: crandmck Date: Fri, 14 Mar 2014 10:28:31 -0700 Subject: [PATCH 5/9] Improvements to JSDoc comments --- lib/models/email.js | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/models/email.js b/lib/models/email.js index cc351c92..3950db6e 100644 --- a/lib/models/email.js +++ b/lib/models/email.js @@ -18,11 +18,11 @@ var properties = { * * **Properties** * - * - `to` - **{ String }** **required** - * - `from` - **{ String }** **required** - * - `subject` - **{ String }** **required** - * - `text` - **{ String }** - * - `html` - **{ String }** + * - `to` - String (required) + * - `from` - String (required) + * - `subject` - String (required) + * - `text` - String + * - `html` - String * * @class * @inherits {Model} @@ -35,19 +35,24 @@ var Email = module.exports = Model.extend('Email', properties); * * Example Options: * - * ```json + * ```js * { - * from: "Fred Foo ✔ ", // sender address + * from: "Fred Foo ", // sender address * to: "bar@blurdybloop.com, baz@blurdybloop.com", // list of receivers - * subject: "Hello ✔", // Subject line - * text: "Hello world ✔", // plaintext body - * html: "Hello world ✔" // html body + * subject: "Hello", // Subject line + * text: "Hello world", // plaintext body + * html: "Hello world" // html body * } * ``` * * See https://github.com/andris9/Nodemailer for other supported options. * - * @param {Object} options + * @options {Object} options See below + * @prop {String} from Senders's email address + * @prop {String} to List of one or more recipient email addresses (comma-delimited) + * @prop {String} subject Subject line + * @prop {String} text Body text + * @prop {String} html Body HTML (optional) * @param {Function} callback Called after the e-mail is sent or the sending failed */ From f031e79392eca4921521828ce71772e4d99b5ff4 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 17 Mar 2014 10:01:29 -0700 Subject: [PATCH 6/9] Remove the generated flag as the id is set by the before hook --- lib/models/application.js | 2 +- test/model.application.test.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/models/application.js b/lib/models/application.js index 9af732f0..f537559e 100644 --- a/lib/models/application.js +++ b/lib/models/application.js @@ -50,7 +50,7 @@ var PushNotificationSettingSchema = { * Data model for Application */ var ApplicationSchema = { - id: {type: String, id: true, generated: true}, + id: {type: String, id: true}, // Basic information name: {type: String, required: true}, // The name description: String, // The description diff --git a/test/model.application.test.js b/test/model.application.test.js index d7f24df1..5d8a92a5 100644 --- a/test/model.application.test.js +++ b/test/model.application.test.js @@ -20,6 +20,7 @@ describe('Application', function () { assert(app.masterKey); assert(app.created); assert(app.modified); + assert.equal(typeof app.id, 'string'); done(err, result); }); }); From 32a9131004f8a9e3630a10c50f675cc967ad3227 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 17 Mar 2014 14:40:05 -0700 Subject: [PATCH 7/9] Pause the req before checking access See https://github.com/strongloop/loopback-storage-service/issues/7 --- lib/application.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/application.js b/lib/application.js index a7fb511c..acb4f1f2 100644 --- a/lib/application.js +++ b/lib/application.js @@ -279,11 +279,16 @@ app.enableAuth = function() { var modelId = modelInstance && modelInstance.id || req.param('id'); if(Model.checkAccess) { + // Pause the request before checking access + // See https://github.com/strongloop/loopback-storage-service/issues/7 + req.pause(); Model.checkAccess( req.accessToken, modelId, method.name, function(err, allowed) { + // Emit any cached data events that fired while checking access. + req.resume(); if(err) { console.log(err); next(err); From 1aaf6eed359661742f4b9178ae25acd8cd9d19b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 18 Mar 2014 08:43:24 +0100 Subject: [PATCH 8/9] Add test for request pausing during authentication --- test/integration.test.js | 88 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 test/integration.test.js diff --git a/test/integration.test.js b/test/integration.test.js new file mode 100644 index 00000000..7aae5dc0 --- /dev/null +++ b/test/integration.test.js @@ -0,0 +1,88 @@ +var net = require('net'); +describe('loopback application', function() { + it('pauses request stream during authentication', function(done) { + // This test reproduces the issue reported in + // https://github.com/strongloop/loopback-storage-service/issues/7 + var app = loopback(); + setupAppWithStreamingMethod(); + + app.listen(0, function() { + sendHttpRequestInOnePacket( + this.address().port, + 'POST /streamers/read HTTP/1.0\n' + + 'Content-Length: 1\n' + + 'Content-Type: application/x-custom-octet-stream\n' + + '\n' + + 'X', + function(err, res) { + if (err) return done(err); + expect(res).to.match(/\nX$/); + done(); + }); + }); + + function setupAppWithStreamingMethod() { + app.dataSource('db', { + connector: loopback.Memory, + defaultForType: 'db' + }); + var db = app.datasources.db; + + loopback.User.attachTo(db); + loopback.AccessToken.attachTo(db); + loopback.Role.attachTo(db); + loopback.ACL.attachTo(db); + loopback.User.hasMany(loopback.AccessToken, { as: 'accessTokens' }); + + var Streamer = app.model('Streamer', { dataSource: 'db' }); + Streamer.read = function(req, res, cb) { + var body = new Buffer(0); + req.on('data', function(chunk) { + body += chunk; + }); + req.on('end', function() { + res.end(body.toString()); + // we must not call the callback here + // because it will attempt to add response headers + }); + req.once('error', function(err) { + cb(err); + }); + }; + loopback.remoteMethod(Streamer.read, { + http: { method: 'post' }, + accepts: [ + { arg: 'req', type: 'Object', http: { source: 'req' } }, + { arg: 'res', type: 'Object', http: { source: 'res' } } + ] + }); + + app.enableAuth(); + app.use(loopback.token({ model: app.models.accessToken })); + app.use(loopback.rest()); + } + + function sendHttpRequestInOnePacket(port, reqString, cb) { + var socket = net.createConnection(port); + var response = new Buffer(0); + + socket.on('data', function(chunk) { + response += chunk; + }); + socket.on('end', function() { + callCb(null, response.toString()); + }); + socket.once('error', function(err) { + callCb(err); + }); + + socket.write(reqString.replace(/\n/g, '\r\n')); + + function callCb(err, res) { + if (!cb) return; + cb(err, res); + cb = null; + } + } + }); +}); From 2c510a7b2e6b8176a3abdd6c143e283a7e661299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 18 Mar 2014 18:15:24 +0100 Subject: [PATCH 9/9] v1.7.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9608391c..b46085c4 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "Platform", "mBaaS" ], - "version": "1.7.0", + "version": "1.7.1", "scripts": { "test": "mocha -R spec" },