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

Feature: Define security in autorest #4018

Merged
merged 25 commits into from
Apr 21, 2021
Merged

Conversation

timotheeguerin
Copy link
Member

@timotheeguerin timotheeguerin commented Mar 25, 2021

resolves #3718
resolves #3719

Provide a standard way to define security model that generators can support. Documentation here

Todo:

  • Add cli flags that automatically inject those definitions
  • Add documentation
  • Add tests

@timotheeguerin timotheeguerin requested a review from daviwil as a code owner March 25, 2021 23:28
Comment on lines 2215 to 2223
"scopes": {
"type": "array",
"items": {
"type": "string"
}
},
"headerName": {
"type": "string"
}
Copy link
Member

Choose a reason for hiding this comment

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

Scopes we need only for token credential and header name we need only for key credential, so why not move these properties into their respective classes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean have this instead of an array

interface SecuritySchemes {
  aadToken?: AADTokenSecurityScheme;
  azureKey?: AzureKeySecurityScheme;
}

The array worked great in typescript because you had unions but as can't use them this might be better

Copy link
Member

Choose a reason for hiding this comment

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

No, I meant why not move "scopes" property inside AADTokenSecurityScheme and "headerName" inside AzureKeyScheme?

Do we need "scopes" for AzureKeyScheme and "headerName" for AADTokenSecurityScheme?

Copy link
Member Author

Choose a reason for hiding this comment

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

it is already like that but how do you can't represent the union of those 2 types in csharp so I made the parent type include everything

@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


Add the following CLI flags
Pacakge Flag Description
@autorest/core --version:https://tinyurl.com/yg5rsmrn For changes to autorest core.
@autorest/modelerfour --use:https://tinyurl.com/yjua6nhb For changes to modelerfour.

Or with all

autorest --version:https://tinyurl.com/yg5rsmrn --use:https://tinyurl.com/yjua6nhb

or use the following in your autorest configuration:
# For changes to autorest core
version: "https://tinyurl.com/yg5rsmrn"

# For changes to modelerfour
use-extension:
  "@autorest/modelerfour": "https://tinyurl.com/yjua6nhb"

If this build is good for you, give this comment a thumbs up. (👍) And you should run `autorest --reset` again once you're finished testing to remove it.

@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2021
@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2021
@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2021
@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2021
@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2021
@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2021
@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


Add the following CLI flags
Pacakge Flag Description
@autorest/core --version:https://tinyurl.com/yf79nxxu For changes to autorest core.
@autorest/modelerfour --use:https://tinyurl.com/yjtn8qpz For changes to modelerfour.

Or with all

autorest --version:https://tinyurl.com/yf79nxxu --use:https://tinyurl.com/yjtn8qpz

or use the following in your autorest configuration:
# For changes to autorest core
version: "https://tinyurl.com/yf79nxxu"

# For changes to modelerfour
use-extension:
  "@autorest/modelerfour": "https://tinyurl.com/yjtn8qpz"

If this build is good for you, give this comment a thumbs up. (👍) And you should run `autorest --reset` again once you're finished testing to remove it.

@timotheeguerin timotheeguerin added the next minor Tag to add on PR to say they should go in the next minor version label Apr 2, 2021
@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


Add the following CLI flags
Pacakge Flag Description
@autorest/core --version:https://tinyurl.com/yg453w6a For changes to autorest core.
@autorest/modelerfour --use:https://tinyurl.com/yzramus5 For changes to modelerfour.

Or with all

autorest --version:https://tinyurl.com/yg453w6a --use:https://tinyurl.com/yzramus5

or use the following in your autorest configuration:
# For changes to autorest core
version: "https://tinyurl.com/yg453w6a"

# For changes to modelerfour
use-extension:
  "@autorest/modelerfour": "https://tinyurl.com/yzramus5"

If this build is good for you, give this comment a thumbs up. (👍) And you should run `autorest --reset` again once you're finished testing to remove it.

@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


Add the following CLI flags
Pacakge Flag Description
@autorest/core --version:https://tinyurl.com/yfhyccns For changes to autorest core.
@autorest/modelerfour --use:https://tinyurl.com/yfratqhg For changes to modelerfour.

Or with all

autorest --version:https://tinyurl.com/yfhyccns --use:https://tinyurl.com/yfratqhg

or use the following in your autorest configuration:
# For changes to autorest core
version: "https://tinyurl.com/yfhyccns"

# For changes to modelerfour
use-extension:
  "@autorest/modelerfour": "https://tinyurl.com/yfratqhg"

If this build is good for you, give this comment a thumbs up. (👍) And you should run `autorest --reset` again once you're finished testing to remove it.

@timotheeguerin timotheeguerin changed the title [WIP] Feature: Define security in autorest Feature: Define security in autorest Apr 20, 2021
@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


Add the following CLI flags
Pacakge Flag Description
@autorest/core --version:https://tinyurl.com/yea49o87 For changes to autorest core.
@autorest/modelerfour --use:https://tinyurl.com/yf699jc6 For changes to modelerfour.

Or with all

autorest --version:https://tinyurl.com/yea49o87 --use:https://tinyurl.com/yf699jc6

or use the following in your autorest configuration:
# For changes to autorest core
version: "https://tinyurl.com/yea49o87"

# For changes to modelerfour
use-extension:
  "@autorest/modelerfour": "https://tinyurl.com/yf699jc6"

If this build is good for you, give this comment a thumbs up. (👍) And you should run `autorest --reset` again once you're finished testing to remove it.

@timotheeguerin timotheeguerin merged commit a743840 into master Apr 21, 2021
@timotheeguerin timotheeguerin deleted the feature/m4-security branch April 21, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next minor Tag to add on PR to say they should go in the next minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

credential scopes discussion credential flag discussion
2 participants