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

Features/jwt decoder encoder tool update #645

Merged
merged 10 commits into from Sep 2, 2022

Conversation

btiteux
Copy link
Collaborator

@btiteux btiteux commented Aug 27, 2022

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • UI change (please include screenshot!)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

Issues with the JWT Detection.
No JWT Encoder.

Issue Number:

What is the new behavior?

  • Improved JWT Token detection (detect with Authorization Header and Bearer word)
  • Improved UI
  • Added JWT Encoder

Other information

Quality check

Before creating this PR, have you:

  • Followed the code style guideline as described in CONTRIBUTING.md
  • Verified that the change work in Release build configuration
  • Checked all unit tests pass

@veler
Copy link
Collaborator

veler commented Aug 27, 2022

UI feedback:

1

The grid gripper should be vertical, and the title is missing on some text input:

image

Same issue with the icon in Decoder mode.

2

This UI below feels a bit too busy.

image

How about hidding Valid issuers, Valid audiences and Token has expirations under a collapsible view on Token has issuer, Token has audience and Token has expiration like in Smart detection option in the Settings of DevToys?

image

src/Directory.Build.props Show resolved Hide resolved
src/dev/impl/DevToys/Core/QueueWorkerViewModelBase.cs Outdated Show resolved Hide resolved
_validationResult = null;
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kudo for adding 60+ tests :D This is great! I see 8 tests failing on my machine though 🤔

image

image

Do they pass on your machine? If yes, perhaps it's the sign of a race condition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hum these tests Passed in devops too. Can you get the errors?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, they seem to all crash on this line:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hum, that's the problem with the expires date and time zone. I'm checking.

@@ -50,4 +50,5 @@ private async Task TreatComputationQueueAsync()
}
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

extra blank line? :P


if (decodeParameters.ValidateAudience)
{
if (!tokenParameters.ValidAudiences.Any())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, bad habit from work they don't like Count and prefer Any...

Did you tell them that .Any() has to instantiate an Enumerator, which means extra allocation and computation while Count has a O(1) ?

_validationResult = null;
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, they seem to all crash on this line:

image

Copy link
Collaborator

@veler veler left a comment

Choose a reason for hiding this comment

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

It looks perfect to me now ! 😁😁😁
Thank you @btiteux !

@veler veler merged commit 3b0be14 into main Sep 2, 2022
@veler veler deleted the features/jwt-decoder-encoder-tool-update branch September 2, 2022 16:09
veler added a commit that referenced this pull request Mar 31, 2023
* added jwt encoder and jwt decoder signature verification

* reworked jwt decoder / encoder

* updated following comments

* fixed small bug on ui

* Fixed a bug in CustomTextBox

* update token date to use UTC

* updated for some small ui fixes

Co-authored-by: Etienne BAUDOUX <ebaudoux@velersoftware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved Smart Detection for JWT tokens JWT Encoder JWT Decoder improvement
2 participants