Add authentication validation to the JSON schema
Why make this change?
Closes #2643
The JSON schema does not contain a list of auth providers, nor does it validate the use of the jwt property. This results in a suboptimal developer experience. By adding this list and validation, developers have an easier time learning about all the options and can be more productive.
What is this change?
This change modifies the JSON schema with a list of providers and validation of the providers and the jwt property.
StackOverflow helped me with the finer details of the validation.
I also added tests to validate my changes.
How was this tested?
- [ ] Integration Tests
- [x] Unit Tests
Sample Request(s)
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
@Aniruddh25 I've processed your comments.
I took a look at the failures of the CI pipeline of the last commits. They seem to be unrelated to my changes: The task has failed because you are using Ubuntu 24.04 or later without mono installed. See https://aka.ms/nuget-task-mono for more information..
Let's hope that a re-run will make them go green!
@RubenCerna2079 Could you perform a re-run on this PR :)
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
I see I broke some config tests. I'll hopefully have time to look into this next week!
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
Hi! I sadly haven't been able to work on this issue this week. I'll be moving house during the next few weeks. I probably won't have time to look until this until the end of May/beginning of June.
If the team wants this merged, I'll sadly have to ask them to fix these tests. Otherwise I will fix them when I have time.
@Aniruddh25 @RubenCerna2079
@sander1095 thank you for your update. This PR can wait when you get more time to work on it. We appreciate your contributions and look forward to when you have updates on this.
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
Hi @Aniruddh25 and @RubenCerna2079 !
I've found some to look into this PR. I've updated it. The failed unit tests now run correctly on my machine!
There's 1 impactful change you should be aware of: I decided to move away from NJsonSchema and use JsonSchemaNet instead. The former doesn't validate oneof + const correctly, which we use for letting the user decide a specific auth provider. JsonSchemaNet DOES support this. If you disagree with this change, we could stay on NJsonSchema and use anyof + const instead, but this is "less" correct as it would indicate multiple auth providers should be supported, which is not the case..
I think it's worth a try to run the pipelines again and see the results!
If the integration fail again, please post the stacktraces. I can't access them as I get a 403 error when trying to see more details in Azure Pipelines.
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
@sander1095 I just saw that some of the tests related to the config validation are failing, if you need any help with that, please let me know.
@RubenCerna2079 That's annoying. Last time I checked they passed on my machine, I'll have another look.
My sincere apologies for this!
Also: I see integration tests fail. I can not inspect the failures as I dont have permission for Azure DevOps. Can you share the test failure reasons/stracktraces?
@sander1095 I just sent the details through teams
Hey @RubenCerna2079 ! Good news!
I just fixed the unit tests by switching from JsonSchema.Net to Newtonsoft.Json.Schema. This package fixes the issues I had with NJsonSchema AND it actually has functioning error reporting.
This package seems to be a much better fit as the project already has a dependency on Newtonsoft.Json. It also allowed me to restore the LineNumber/LinePosition message.
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
I can now see the integration test failure reasons! Some of them fail because the package migration changed the JSON Schema result messages. I can update this soon.
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run