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

Show additional JWT info #864

Merged
merged 4 commits into from
Jul 4, 2023
Merged

Conversation

micahmo
Copy link

@micahmo micahmo commented Jun 23, 2023

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • New feature or enhancement
  • 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?

Issue Numbers: #607 and #670

What is the new behavior?

This PR adds the ability to show additional information about a decoded JWT. Key features are...

  • Each top-level property of the JWT is shown in a table with the key in one column and the value in another.
  • The values of date/time fields are shown in human-readable formats (instead of timestamp/long).
  • Clicking the row for a given field will display an explanation about that field (if it is a known/documented JWT claim).

Other information

There are several things to mention about this implementation.

  • In order to inject the information, I went with a slightly different approach than was suggested. Rather than having the parent control inject the info button, it is part of the standard CodeEditor. However the info control itself can be set via a DependencyProperty, and the CodeEditor will show whatever's there.
  • I struggled with how to disply the claim descriptions. They're usually very long, and I had trouble fitting them in a third column in the table. My options were (a) allow the grid to expand as wide as it wants, putting the info on one line and marking it hard to read, or (b) keep the grid width fixed and make the description wrap, which ended causing each row to consume too much vertical space. I thought about some other alternatives (truncating the description until clicked), but the bigger issue here is an apparent bug in the Toolkit DataGrid which causes rows taller than one line of text to have trouble rendering. I can upload a video of this if anyone's curious.
    Luckily, I ran across Row Details and I thought it would be the perfect compromise of having a nice clean grid initially with easy access to the descriptions. One quirk here is that I would expect clicking the row again to toggle the details off, but it doesn't. Maybe some special code can be added in the future to do that.
  • Along the same lines, I noticed that the grid doesn't respond perfectly to live theme changes. You have to scroll up and down to get all the rows to convert. I'm guessing this is another bug and maybe related to virtualization. However, I think it's ok since it's a rare case (changing theme while this control is open), and it eventually fixes itself.
  • Finally, there is some funny handling related to sizing. In addition to the technique used in the CustomTextBox (a Border whose size controls the TextBox's size), I also had to add another trick. When expanding or collapsing the addiitonal info view, I quickly toggle on the CodeEditor. Without this, the Border has the wrong size. This does occasionally create a very small visual artifact, but it's the only way I could get the sizing to be solid.
DevToys-JwtInfo-Demo.mp4

Quality check

Before creating this PR:

  • Did you follow the code style guideline as described in CONTRIBUTING.md
  • Did you build the app and test your changes?
  • Did you check for accessibility? On Windows, you can use Accessibility Insights for this.
  • Did you verify that the change work in Release build configuration
  • Did you verify that all unit tests pass
  • If necessary and if possible, did you verify your changes on:
    • Windows
    • macOS (DevToys 2.0)
    • Linux (DevToys 2.0)

@veler
Copy link
Collaborator

veler commented Jun 28, 2023

Alright, I (finally) gave a try at the PR (so sorry for taking so long, focusing on DevToys 2.0 + my personal life on the side takes a lot of time).

It remains one "tiny" sizing issue on which I'm a little picky.

image

Here is the issue. and it sounds related to everything we mentioned previously:

Recording.2023-06-27.215601.mp4

I can also still reproduce the issue you demoed in the video in the previous comment, unfortunately. Do you reproduce it on your side too beside the RunIdleAsync ?

In the end, this PR is already super great and I don't think it's a blocker for merging it, but it would be amazing if we could address is before though (just to make it perfect). What do you think?

@micahmo
Copy link
Author

micahmo commented Jun 28, 2023

so sorry for taking so long, focusing on DevToys 2.0 + my personal life on the side takes a lot of time

No worries whatsoever, I appreciate the tool and having the chance to contribute just a little. 😊 BTW, after this and #859 I would probably be done for a little while if you wanted to make a release (I'm not trying to dictate your release schedule or anything like that, just saying I added most of the things I wanted 😆).

It remains one "tiny" sizing issue on which I'm a little picky.

The issue you demonstrated is actually unrelated to anything we've talked about so far. In this case, you are giving the DataGrid more room than it needs, and it doesn't seem to have any notion of expanding to be taller than the number of rows. Therefore, if the number of rows is very small, it will not occupy the total vertical space. This can be seen without resizing just by maximizing the window and providing a small JWT.

image

Let me see if there are any more hacky solutions I can come up with for this. 😆

@veler
Copy link
Collaborator

veler commented Jun 28, 2023

Oh what I was referring to in fact is that the code editor on the left doesn't shrink at all as we make the window height smaller when the data grid is displayed, but does when the code editor on the right is displayed.

@micahmo
Copy link
Author

micahmo commented Jun 28, 2023

Ah I totally missed that. I was so focused on the right side! Yes, I can reproduce that too. Good catch! I will take a look.

@micahmo
Copy link
Author

micahmo commented Jun 28, 2023

@veler I've been meaning to ask you, when you run DevToys in debug mode, does it take a long time for the Monaco editor to load? For example, when I click on the JWT tool, it takes about 1.25 minutes while the editors spin until the UI is responsive. Is this normal or maybe I'm doing something wrong?

@veler
Copy link
Collaborator

veler commented Jun 28, 2023

@veler I've been meaning to ask you, when you run DevToys in debug mode, does it take a long time for the Monaco editor to load? For example, when I click on the JWT tool, it takes about 1.25 minutes while the editors spin until the UI is responsive. Is this normal or maybe I'm doing something wrong?

Not sure if "normal" but I always hit it too. There's a LOT happening to load Monaco due to the whole interop we need to do between the C# code and the JavaScript int he WebView. Even without debugger attached it takes 1-2 seconds to load (and we have loads of complain about it). My recommendation is that, when possible, start DevToys without Debugger, then attach the debugger through Debug > Attach to Process in Visual Studio.

Good news though: in DevToys 2.0, it takes ~50ms to load, whether you are with the debugger attached or not 🙌

@micahmo
Copy link
Author

micahmo commented Jul 3, 2023

Hi @veler, thanks for your patience on this. I have been working off and on for the past few days, but unfortunately, I can't get it to work as desired. The closest I could get was pulling the DataGrid into the parent ToolPage and then setting its height to some percentage less than 100% of the ActualHeight of the other CodeEditor (payload). Unfortunately, as soon as the heights match, the DataGrid takes more space, causing the CodeEditor to take more space, and they both keep expanding until the DataGrid is full height.

I believe we have the following options at this point.

  1. Accept the PR as-is. It is not perfect, but maybe the new functionality will outweigh the minor UI issue.
  2. Abandon the PR. I will take no offense at this as I totally understand that we want things to be perfect.
  3. You or someone else can take a look. There's a good chance I missed something and we need a more expert eye.
  4. Use an alternative control. Actually, my first idea was to format the data as an ASCII table (using a library) and put it in a TextBox, which might size better.

Again, I hate giving up, but I would rather move on than sink too much time into this one. 😊

@veler
Copy link
Collaborator

veler commented Jul 4, 2023

Hi @veler, thanks for your patience on this. I have been working off and on for the past few days, but unfortunately, I can't get it to work as desired. The closest I could get was pulling the DataGrid into the parent ToolPage and then setting its height to some percentage less than 100% of the ActualHeight of the other CodeEditor (payload). Unfortunately, as soon as the heights match, the DataGrid takes more space, causing the CodeEditor to take more space, and they both keep expanding until the DataGrid is full height.

I believe we have the following options at this point.

  1. Accept the PR as-is. It is not perfect, but maybe the new functionality will outweigh the minor UI issue.
  2. Abandon the PR. I will take no offense at this as I totally understand that we want things to be perfect.
  3. You or someone else can take a look. There's a good chance I missed something and we need a more expert eye.
  4. Use an alternative control. Actually, my first idea was to format the data as an ASCII table (using a library) and put it in a TextBox, which might size better.

Again, I hate giving up, but I would rather move on than sink too much time into this one. 😊

No worries :D Thank you so much for trying hard on it. I took a brief look and I admit I don't see an easy solution either.
I will approve and merge this PR. I think the benefits outweigh this detail. Worst case, we will get some issues opened and will act on it.

Thank you so much for this again! We love your contributions 😍

@veler veler merged commit b9533c3 into DevToys-app:main Jul 4, 2023
@micahmo micahmo deleted the feature/jwt-info branch July 5, 2023 02:42
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.

JWT Decoder show timestamp as a DateTime tooltip Explain claims of JWT token like jwt.ms
2 participants