Skip to content
This repository has been archived by the owner on Aug 7, 2021. It is now read-only.

client-credential cert-bad-cert failing #71

Closed
jmspring opened this issue Aug 9, 2015 · 6 comments
Closed

client-credential cert-bad-cert failing #71

jmspring opened this issue Aug 9, 2015 · 6 comments
Assignees
Labels
Milestone

Comments

@jmspring
Copy link

jmspring commented Aug 9, 2015

108 passing (996ms)
1 failing

  1. client-credential cert-bad-cert:
  AssertionError: Unexpected error messageerror:0906D06C:PEM routines:PEM_read_bio:no start line
  + expected - actual

  -false
  +true

  at test/client-credential.js:279:7
  at TokenRequest.getTokenWithCertificate (lib/token-request.js:507:5)
  at AuthenticationContext.<anonymous> (lib/authentication-context.js:359:18)
  at lib/authentication-context.js:201:19
  at Authority._getOAuthEndpoints (lib/authority.js:223:5)
  at lib/authority.js:251:14
  at Authority._validateViaInstanceDiscovery (lib/authority.js:195:5)
  at Authority.validate (lib/authority.js:245:10)
  at AuthenticationContext._acquireToken (lib/authentication-context.js:196:19)
  at AuthenticationContext.acquireTokenWithClientCertificate (lib/authentication-context.js:357:8)
  at Context.<anonymous> (test/client-credential.js:277:13)
@aj-michael
Copy link
Contributor

Hmmm, can you confirm that this is occurring for you on master? I am not currently able to recreate this failure.

@jmspring
Copy link
Author

Yes. On a Linux VM. Details below:

jims@ azurespiel:~/tmp/azure-activedirectory-library-for-nodejs$ uname -a
Linux azurespiel 3.19.0-25-generic #26-Ubuntu SMP Fri Jul 24 21:17:31 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

jims@ azurespiel:~/tmp/azure-activedirectory-library-for-nodejs$ cat /etc/issue
Ubuntu 15.04 \n \l

Node version: v0.12.7


113 passing (944ms)
1 failing

  1. client-credential cert-bad-cert:
  AssertionError: Unexpected error messageerror:0906D06C:PEM routines:PEM_read_bio:no start line
  + expected - actual

  -false
  +true

  at test/client-credential.js:279:7
  at TokenRequest.getTokenWithCertificate (lib/token-request.js:534:5)
  at AuthenticationContext.<anonymous> (lib/authentication-context.js:359:18)
  at lib/authentication-context.js:201:19
  at Authority._getOAuthEndpoints (lib/authority.js:223:5)
  at lib/authority.js:251:14
  at Authority._validateViaInstanceDiscovery (lib/authority.js:195:5)
  at Authority.validate (lib/authority.js:245:10)
  at AuthenticationContext._acquireToken (lib/authentication-context.js:196:19)
  at AuthenticationContext.acquireTokenWithClientCertificate (lib/authentication-context.js:357:8)
  at Context.<anonymous> (test/client-credential.js:277:13)

npm ERR! Test failed. See above for more details.

@aj-michael
Copy link
Contributor

Ok, I upgraded from node version 0.10.25 to 0.12.7 and I am now able to reproduce this issue. Will investigate.

@jmspring
Copy link
Author

Thanks Adam.

On Wed, Aug 12, 2015 at 1:29 PM, Adam Michael [email protected]
wrote:

Ok, I upgraded from node version 0.10.25 to 0.12.7 and I am now able to
reproduce this issue. Will investigate.


Reply to this email directly or view it on GitHub
#71 (comment)
.

@aj-michael
Copy link
Contributor

Hmmm. So this error is being thrown by a library jws that we use. That library also fails a unit test for the exact same reason on 0.12.7. It looks like around version 0.11.11, Nodejs made a breaking change in crypto.js that determines what to do with bad certificates. I can't find it in the change logs, but here is one of the maintainers of nodejs acknowledging it:

nodejs/node#408

We will probably just have to catch this error and rethrow, but I should talk to some people about it first.

In case anyone else is curious, the followign code returns undefined in 0.11.10 but throws the error in 0.11.11:

require('jws').sign({ header: {'alg': 'RS256'}, payload: {}, secret: 'not a certificate' });

@weijjia
Copy link
Contributor

weijjia commented Sep 12, 2015

Thanks for the investigation Adam has already made. It looks like nodejs did make an intentional change on the error thrown for bad certificates. We should catch the error and re-throw it. Will send a pull request for it.

amishra-dev pushed a commit that referenced this issue Apr 25, 2016
This is a fix for issues #71, #78 , and #80
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants