Updated sample.js and README.md and fixed problems introduced by the …

…callback changes
snoble · Jun 21, 2011 · 1b298d7 · 1b298d7
1 parent e637622
commit 1b298d7
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 34 deletions.
diff --git a/README.md b/README.md
@@ -48,11 +48,16 @@ using OpenID for node.js for authentication:
               var identifier = query.openid_identifier;
 
               // Resolve identifier, associate, and build authentication URL
-              relyingParty.authenticate(identifier, false, function(authUrl)
+              relyingParty.authenticate(identifier, false, function(error, authUrl)
                   {
-                    if (!authUrl)
+                    if (error)
                     {
-                      res.writeHead(500);
+                      res.writeHead(200);
+                      res.end('Authentication failed: ' + error);
+                    }
+                    else if (!authUrl)
+                    {
+                      res.writeHead(200);
                       res.end('Authentication failed');
                     }
                     else
@@ -66,10 +71,10 @@ using OpenID for node.js for authentication:
             {
                 // Verify identity assertion
                 // NOTE: Passing just the URL is also possible
-                relyingParty.verifyAssertion(req, function(result)
+                relyingParty.verifyAssertion(req, function(error, result)
                 {
                   res.writeHead(200);
-                  res.end(result.authenticated 
+                  res.end(!error && result.authenticated 
                       ? 'Success :)'
                       : 'Failure :(');
                 });

diff --git a/openid.js b/openid.js
@@ -743,9 +743,9 @@ openid.authenticate = function(identifier, returnUrl, realm, immediate, stateles
 
     var providerIndex = -1;
 
-    var chooseProvider = function successOrNext(authUrl)
+    var chooseProvider = function successOrNext(error, authUrl)
     {
-      if(authUrl)
+      if(!error && authUrl)
       {
         var provider = providers[providerIndex];
         if(provider.claimedIdentifier)
@@ -857,10 +857,10 @@ function _requestAuthentication(provider, assoc_handle, returnUrl, realm, immedi
   }
   else if(!returnUrl)
   {
-    throw new Error("No return URL or realm specified");
+    callback('No return URL or realm specified');
   }
 
-  callback(_buildUrl(provider.endpoint, params));
+  callback(null, _buildUrl(provider.endpoint, params));
 }
 
 openid.verifyAssertion = function(requestOrUrl, callback, stateless, extensions)
@@ -891,8 +891,12 @@ openid.verifyAssertion = function(requestOrUrl, callback, stateless, extensions)
     {
       return callback(error, { authenticated: false });
     }
-    _checkSignature(params, function(result)
+    _checkSignature(params, function(error, result)
     {
+      if(error)
+      {
+        return callback(error);
+      }
       if(extensions && result.authenticated)
       {
         for(var ext in extensions)
@@ -974,10 +978,10 @@ function _verifyDiscoveredInformation(params, callback)
         {
           continue;
         }
-        _verifyAssertionAgainstProvider(provider, params, callback);
+        return _verifyAssertionAgainstProvider(provider, params, callback);
       }
 
-      callback('No providers were discovered for the claimed identifier');
+      return callback('No providers were discovered for the claimed identifier');
     });
   });
 }

diff --git a/sample.js b/sample.js
@@ -68,36 +68,48 @@ var server = require('http').createServer(
           var identifier = query.openid_identifier;
 
           // Resolve identifier, associate, and build authentication URL
-          relyingParty.authenticate(identifier, false, function(authUrl)
-              {
-                if (!authUrl)
-                {
-                  res.writeHead(500);
-                  res.end('Authentication failed');
-                }
-                else
-                {
-                  res.writeHead(302, { Location: authUrl });
-                  res.end();
-                }
-              });
+          relyingParty.authenticate(identifier, false, function(error, authUrl)
+          {
+            if(error)
+            {
+              res.writeHead(200);
+              res.end('Authentication failed: ' + error);
+            }
+            else if (!authUrl)
+            {
+              res.writeHead(200);
+              res.end('Authentication failed');
+            }
+            else
+            {
+              res.writeHead(302, { Location: authUrl });
+              res.end();
+            }
+          });
         }
         else if(parsedUrl.pathname == '/verify')
         {
-            // Verify identity assertion
-            // NOTE: Passing just the URL is also possible
-            relyingParty.verifyAssertion(req, function(result)
+          // Verify identity assertion
+          // NOTE: Passing just the URL is also possible
+          relyingParty.verifyAssertion(req, function(error, result)
+          {
+            res.writeHead(200);
+
+            if(error)
+            {
+              res.end('Authentication failed: ' + error);
+            }
+            else
             {
               // Result contains properties:
               // - authenticated (true/false)
-              // - error (message, only if not authenticated)
               // - answers from any extensions (e.g. 
               //   "http://axschema.org/contact/email" if requested 
               //   and present at provider)
-              res.writeHead(200);
               res.end((result.authenticated ? 'Success :)' : 'Failure :(') +
                 '\n\n' + JSON.stringify(result));
-            });
+            }
+          });
         }
         else
         {

diff --git a/test/openid_fast_tests.js b/test/openid_fast_tests.js
@@ -33,7 +33,7 @@ exports.testVerificationUrl = function(test)
   openid.verifyAssertion('http://fu', function(error, result)
   {
     assert.ok(!times++);
-    assert.ok(!result.authenticated);
+    assert.ok(!result || !result.authenticated);
     test.done();
   });
 }
@@ -47,7 +47,7 @@ exports.testVerificationCancel = function(test)
       function(error, result)
   {
     assert.ok(!times++);
-    assert.ok(!result.authenticated);
+    assert.ok(!result || !result.authenticated);
     test.done();
   });
 }
@@ -63,7 +63,7 @@ exports.testVerificationUrlUsingRelyingParty = function(test)
 
   rp.verifyAssertion('http://fu', function(error, result)
   {
-    assert.ok(!result.authenticated);
+    assert.ok(!result || !result.authenticated);
     test.done();
   });
 }