usnistgov / ACVP-Server

A repository tracking releases of NIST's ACVP server. See www.github.com/usnistgov/ACVP for the protocol.
51 stars 18 forks source link

Error refreshing token without information #287

Open AlexThurston opened 1 year ago

AlexThurston commented 1 year ago

When trying to refresh a test session access token, I am getting a 500 failure from the endpoint. While it's possible that I am doing incorrectly, the error that is returned with the 500 does not provide any information on what the incorrect usage might be.

[
  {
    "acvVersion": "1.0"
  },
  {
    "error": "Internal service error. Contact service provider."
  }
]

environment Demo

Endpoint in which the error is experienced https://demo.acvts.nist.gov/acvp/v1/login/refresh POST

Expected behaviour I would be able to refresh a token, or get an actionable if used incorrectly.

Additional context Obviously not posting the access token since I believe these are meant to protected.

livebe01 commented 1 year ago

Are you performing a POST? or a GET?

AlexThurston commented 1 year ago

Sorry, that was a typo, I was using a POST. I'll edit original message.

livebe01 commented 1 year ago

Can you provide the test session id(s) for which you're attempting to refresh the access token(s)?

AlexThurston commented 1 year ago

Sure. Test session 443249 for example. This is a test session created on DEMO. The token I am trying to refresh has this for info:

iat: 1696273943 -> Monday, October 2, 2023 3:12:23 PM [GMT-04:00] exp: 1696275743 -> Monday, October 2, 2023 3:42:23 PM [GMT-04:00]

As a note to what might be going on here, it's possible that this token ha already been refreshed at another time already and that once a given token has been refreshed, it cannot be refreshed again. I'd be surprised if this was the case as this is the first time that I've run into this situation.

Further, if that is the case, then the error message isn't quite clear as it suggests this is an error on ACVP's end and not the user's end.

livebe01 commented 1 year ago

Hmmm... 443249 is marked as cancelled on our side.

AlexThurston commented 1 year ago

Interesting. OK. Let me dig on my side a little bit.

That said, what is the expected behaviour when I have an access token t1 which is expired. And I refresh the token successfully to t2 and then some time later I follow the refresh process again using t1 as the token? Should this successfully give me another access token t3?

livebe01 commented 1 year ago

It should, yes.

AlexThurston commented 1 year ago

OK. How about test session 444427

Here is part of what is in the JWT that I'm trying to refresh which expired 4 hours ago

{
  "tsId": "444427",
  "vsId": "[1868852,1868853,1868854,1868855]",
  "nbf": 1696436101,
  "exp": 1696437901,
  "iat": 1696436101,
  "iss": "NIST ACVP DEMO"
}

Response body:

[
  {
   "acvVersion": "1.0"
  },
  {
   "error": "Internal service error. Contact service provider."
  }
]
AlexThurston commented 1 year ago

I tried this twice around Wed 4 Oct 2023 16:33:59 EDT if that helps with checking logs.

livebe01 commented 1 year ago

Thanks. That's helpful. It looks like the "sub"s do not match between your client cert and your JWT. Your client cert sub is/begins w/ E=info@lightshipsec.com. Your JWT sub is/begins w/ E=alex.thurston@lightshipsec.com.

AlexThurston commented 1 year ago

Doh! Of course! Sorry for wasting your time. That said, this bring two thoughts up which might be something to look at.

The first is that it might be nice to improve the error message to both indicate that this was a me problem and not a you problem since current the error suggests otherwise.

The second would perhaps be the idea of having credentials live as part of a group or federation. The idea here being that a set of credentials could update a test session's access token rather than it be tied directly to a single set of credentials. This one is just a thought but would certainly help in larger organizations. I believe that perhaps AMVP is implementing this (although I might be wrong). But it does seem like it would be a win for the user regardless.

livebe01 commented 12 months ago

Sure, I agree. We should make a better error message for this. I can't do that right now, but that should be something we can do.

For #2, that's probably something Chris has thought about. I'll ping him to see if he has anything to mention.