data-api-builder icon indicating copy to clipboard operation
data-api-builder copied to clipboard

Add authentication validation to the JSON schema

Open sander1095 opened this issue 9 months ago • 10 comments

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)

image

image

image

sander1095 avatar Apr 09 '25 09:04 sander1095

/azp run

RubenCerna2079 avatar Apr 11 '25 18:04 RubenCerna2079

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar Apr 11 '25 18:04 azure-pipelines[bot]

@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!

sander1095 avatar Apr 14 '25 14:04 sander1095

@RubenCerna2079 Could you perform a re-run on this PR :)

sander1095 avatar Apr 15 '25 08:04 sander1095

/azp run

Aniruddh25 avatar Apr 16 '25 17:04 Aniruddh25

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar Apr 16 '25 17:04 azure-pipelines[bot]

I see I broke some config tests. I'll hopefully have time to look into this next week!

sander1095 avatar Apr 17 '25 21:04 sander1095

/azp run

RubenCerna2079 avatar Apr 21 '25 16:04 RubenCerna2079

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar Apr 21 '25 16:04 azure-pipelines[bot]

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 avatar Apr 25 '25 16:04 sander1095

@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.

Aniruddh25 avatar Apr 29 '25 21:04 Aniruddh25

/azp run

Aniruddh25 avatar May 10 '25 00:05 Aniruddh25

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar May 10 '25 00:05 azure-pipelines[bot]

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.

sander1095 avatar May 15 '25 12:05 sander1095

/azp run

RubenCerna2079 avatar May 27 '25 17:05 RubenCerna2079

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar May 27 '25 17:05 azure-pipelines[bot]

@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 avatar May 27 '25 18:05 RubenCerna2079

@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 avatar May 27 '25 18:05 sander1095

@sander1095 I just sent the details through teams

RubenCerna2079 avatar May 27 '25 18:05 RubenCerna2079

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.

sander1095 avatar May 27 '25 20:05 sander1095

/azp run

RubenCerna2079 avatar May 27 '25 20:05 RubenCerna2079

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar May 27 '25 20:05 azure-pipelines[bot]

/azp run

Aniruddh25 avatar Jun 05 '25 23:06 Aniruddh25

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar Jun 05 '25 23:06 azure-pipelines[bot]

/azp run

Aniruddh25 avatar Jun 10 '25 20:06 Aniruddh25

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar Jun 10 '25 20:06 azure-pipelines[bot]

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.

sander1095 avatar Jun 11 '25 08:06 sander1095

/azp run

RubenCerna2079 avatar Jun 13 '25 00:06 RubenCerna2079

Azure Pipelines successfully started running 6 pipeline(s).

azure-pipelines[bot] avatar Jun 13 '25 00:06 azure-pipelines[bot]

/azp run

RubenCerna2079 avatar Jun 17 '25 22:06 RubenCerna2079