Skip to content

Conversation

@johnjarrard
Copy link

… clarity). Added Fn_Split to use with fn_JWT_Decode

…clarity). Added Fn_Split to use with fn_JWT_Decode
@chrisblatchley
Copy link
Contributor

Looks great. I no longer work for this company so I can't merge it in. Also, the fn_ prefix isn't how Veracross does user defined functions so it unfortunately might not get merged in for that reason. However, I'll star your fork.

@sixfeetover
Copy link
Member

@johnjarrard Thanks! I'd gladly pull in the new functions; however, as Chris mentioned we don't prefix functions with fn_ here. We also use lowercase for keywords - updating the formatting of Split to match the rest of the project would be helpful.

If you want to submit just the enhanced functionality without the fn_ rename, I'll merge.

Copy link

@Imran-imtiaz48 Imran-imtiaz48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR introduces several improvements and refactoring in the jwt.sql file. The changes include:

  • Creation of new, consistently named scalar functions (fn_Split, fn_XmlToJson, fn_HMAC, fn_Base64, fn_Base64ToBinary, fn_JWT_Encode, fn_JWT_Decode)
  • Renaming all functions to use the fn_ prefix for clarity and consistency
  • Addition of a new fn_Split function for string splitting
  • Addition of a new fn_Base64ToBinary function with URL-safe decoding support
  • Implementation of a new fn_JWT_Decode function for JWT parsing and validation
  • Refactor of all function calls in the JWT encode/decode logic to use the new names
  • Dropping and re-granting execute permissions on all updated functions
  • Updated example usage (commented out) for the new function names

Strengths

  • Modularity & Consistency: The use of the fn_ prefix improves naming conventions, making the codebase easier to navigate and maintain.
  • New Functionality: The addition of fn_Split, fn_Base64ToBinary, and fn_JWT_Decode extends the capabilities of this SQL JWT implementation, making it more feature-complete.
  • Security: The decode function properly verifies the signature, returning an error JSON object if verification fails.
  • Clean-up: Old function names are dropped before recreating, preventing duplicates or conflicts.
  • Permissions: The script consistently grants execute permissions after each function definition.

Suggestions

  1. Documentation:

    • Consider adding more comments or a brief documentation section at the top of the file explaining the purpose of each function, especially the new ones.
    • Document the expected inputs/outputs for each function for easier onboarding of new contributors.
  2. Error Handling:

    • In fn_JWT_Decode, the error message is hardcoded as a JSON string. You might want to consider standardizing error responses across all functions for consistency.
  3. Testing:

    • If not already present, add unit tests or usage examples for the new functions (especially fn_Split, fn_Base64ToBinary, and fn_JWT_Decode).
    • Ensure the JWT decode/encode roundtrip works as expected with various payloads, secrets, and edge cases.
  4. Performance:

    • For large payloads or high throughput scenarios, review the performance of string operations (especially in fn_Split). If this will be used in production with significant volume, consider potential optimizations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants