Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Senseless Default-Parameter in new TMVCJWTAuthenticationMiddleware Constructor #573

Closed
DeddyH opened this issue Sep 10, 2022 · 0 comments
Closed
Assignees
Labels
accepted Issue has been accepted and inserted in a future milestone

Comments

@DeddyH
Copy link

DeddyH commented Sep 10, 2022

In the "new" constructor the fourth parameter AConfigClaims of type TJWTClaimsSetup is default nil, but if you leave it blank you will get an exception in OnBeforeRouting. To solve this problem I suggest one of the following solutions:

  1. Make it mandatory. This would require swapping params for uniqueness, e.g.
constructor Create(
  AConfigClaims: TJWTClaimsSetup; 
  AAuthenticationHandler: IMVCAuthenticationHandler; 
  ASecret: string = 'D3lph1MVCFram3w0rk';
  ALoginURLSegment: string = '/login'; 
  AClaimsToCheck: TJWTCheckableClaims = []; 
  ALeewaySeconds: Cardinal = 300); overload; virtual;
  1. Leave the signature as is, but set up a default one if nil, e.g.
constructor TMVCJWTAuthenticationMiddleware.Create(AAuthenticationHandler: IMVCAuthenticationHandler;
  ASecret, ALoginURLSegment: string; AConfigClaims: TJWTClaimsSetup; AClaimsToCheck: TJWTCheckableClaims;
  ALeewaySeconds: Cardinal);
begin
  inherited Create;
  FAuthenticationHandler := AAuthenticationHandler;
  FSetupJWTClaims := AConfigClaims;
  if not Assigned(FSetupJWTClaims) then
    FSetupJWTClaims := procedure(const JWT: TJWT)
	  begin
	    // setup some reasonable claims here
	  end;
  ...
end;
@danieleteti danieleteti self-assigned this Sep 10, 2022
@danieleteti danieleteti added the accepted Issue has been accepted and inserted in a future milestone label Sep 10, 2022
@danieleteti danieleteti added this to the 3.2.2-nitrogen milestone Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Issue has been accepted and inserted in a future milestone
Projects
None yet
Development

No branches or pull requests

2 participants