sitemule / ILEastic

Embedded application server for ILE on IBM i
Apache License 2.0
58 stars 29 forks source link

jwt_isExpired Not Working #78

Closed rkanemeier closed 4 years ago

rkanemeier commented 4 years ago

(please forgive me if I break any protocols for reporting issues...this is my first time contributing to a project in github)

Procedure jwt_isExpired in module source /ileastic/plugins/jwt/jwt.rpgle has 2 issues:

  1. The parameter pPayload is not being parsed to extract the exp value. Instead, variable payload is parsed. I do not see any pointer references or any eval statements to move pPayload into payload.

  2. When the payload variable is parsed into the json variable, and the exp value is extracted using the json_getInt() procedure, the results "can be" a -1. The code immediately following the json_getInt() checks of the exp value is >= 0. Since the expired variable defaults to *off (or not expired), then if the json_getInt() procedure fails to return an expiration value, the procedure will always return not expired.

These 2 issues are causing jwt_isExpired to erroneously return "not expired" for jwt's that are expired.

I know how to fix the issues but have not kept "up-to-speed" on how to use Github to change the code.

json = json_parseString(pPayload); exp = json_getInt(json : 'exp' : -1); if (exp >= 0); expTimestamp = UNIX_EPOCH_START + %seconds(exp + %int(offsetSeconds)); expired = (now >= expTimestamp); else; expired = *on; endif;

m1h43l commented 4 years ago

Hi,

thanks for taking a look at the code. I think this issue will be fixed with the currently open pull request #77 . Please check the code at the pull request or my fork of ILEastic.

Thanks

Mihael

rkanemeier commented 4 years ago

Thanks @m1h43l ! I looked at your code and wanted to make sure that the procedure that returns expired considers what happens when json_getInt(json : 'exp' : -1) returns -1. In your fork, the code still does not handle returning *on when json_getInt(json : 'exp' : -1) returns -1 (or am i missing something).

m1h43l commented 4 years ago

@rkanemeier : As you can see there is no else branch anymore. So expired will keep its *off value if there is no exp attribute in the payload.

  dcl-s expired ind inz(*off);

  ...

  exp = json_getInt(json : 'exp' : -1);

  if (exp >= 0);
    expTimestamp = UNIX_EPOCH_START + %seconds(exp + %int(offsetSeconds));
    expired = (now >= expTimestamp);
  endif;

  return expired;
rkanemeier commented 4 years ago

@m1h43l gotcha. i think i understand your architectural direction you want to keep with this procedure. it's not meant to validate whether exp exists. It only validates "when it exists" whether it is expired. understood.

NielsLiisberg commented 4 years ago

@m1h43l and @rkanemeier - Are you both happy ? can we close it now?

m1h43l commented 4 years ago

I think we can close this.