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

Add Authorization Code Grant OAuth security test #457

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

joeyleeeeeee97
Copy link
Contributor

Secure Communication

Test the data that travels across a network can not be accessed by someone who is not the intended recipient, and test varities combination of
cryptography and application protocol works together.

  • User Story

Assume server is an small application needs to read data from github, and github access requires a token to use its public API.
Normally, users won't trust the 3rd party application with their account and password, so we need to redirect user to the github login website
and callback to app with logined token.
But still, token is very important and shouldn't be transported via plain text, so we introduce Authorization Code and let the application backend
to request the token with specific app secret.

  • But the Authorization Code is still text, what if someone try to get a token via that code?

OAuth center must verify code and app secret to ensure only the application could get token.

  • Is this a common usage?

Authorization Code Grant
https://www.oauth.com/oauth2-servers/server-side-apps/authorization-code/

@joeyleeeeeee97
Copy link
Contributor Author

#453

@joeyleeeeeee97 joeyleeeeeee97 force-pushed the oauth branch 2 times, most recently from 6cbe2ce to 26bbd38 Compare September 22, 2021 02:14
@tellison
Copy link

I'll let the test experts speak to the details of this, but it looks like a great contribution to me! Thank you.

Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @joeyleeeeeee97 ! I am still reviewing this, but thought I would submit some initial comments, which are minor grammar/spelling in comments. There are also a couple of binary files in this PR. Do you mean to include the .DS_store file?

As part of a review, reviewers will want to run some Grinder jobs in Jenkins. For this and for inclusion into our regular automated testing, there will need to be a complementary PR in the aqa-tests repo.

Do you plan to create a PR in aqa-tests to hook this test into our AQA automation? Likely in its own subdirectory called security inside https://github.com/adoptium/aqa-tests/tree/master/system with a build.xml to instruct how to fetch/build this material and playlist.xml for the test target to execute.

Comment on lines 17 to 18
Test the data that travels across a network can not be accessed by someone who is not the intended recipient, and test varities combination of
cryptography and application protocol works together.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Test the data that travels across a network can not be accessed by someone who is not the intended recipient, and test varities combination of
cryptography and application protocol works together.
Test the data that travels across a network can not be accessed by someone who is not the intended recipient, and test various combinations of cryptography and application protocols work together.


## Overview
### Security Tests
Security technology includes a large set of APIs, tools, and implementations of commonly-used security algorithms, mechanisms, and protocols.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Security technology includes a large set of APIs, tools, and implementations of commonly-used security algorithms, mechanisms, and protocols.
Security technology includes a large set of APIs, tools, and implementations of commonly used security algorithms, mechanisms, and protocols.

##### OAuth
An Application using 'Authorization Code Grant', which contains
- a Client(performs a login)
- Server(Proxy login to Auth Centor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Server(Proxy login to Auth Centor)
- Server(Proxy login to Auth Center)

An Application using 'Authorization Code Grant', which contains
- a Client(performs a login)
- Server(Proxy login to Auth Centor)
- Auth Center(authenticat user login)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Auth Center(authenticat user login)
- Auth Center(authenticate user login)

- User Story
> Assume server is an small application needs to read data from github, and github access requires a token to use its public API.
> Normally, users won't trust the 3rd party application with their account and password, so we need to redirect user to the github login website
and callback to app with logined token.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
and callback to app with logined token.
and callback to app with login token.

Comment on lines 31 to 32
> But still, token is very important and shouldn't be transported via plain text, so we introduce Authorization Code and let the application backend
> to request the token with specific app secret.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> But still, token is very important and shouldn't be transported via plain text, so we introduce Authorization Code and let the application backend
> to request the token with specific app secret.
> But still, the token is very important and should not be transported via plain text, so we introduce an Authorization Code and let the application backend request the token with specific app secret.

@joeyleeeeeee97
Copy link
Contributor Author

@smlambert Thanks for the suggestions and yes I am working on a PR in aqa-tests to include this test.

@joeyleeeeeee97 joeyleeeeeee97 force-pushed the oauth branch 3 times, most recently from c9fdbb2 to d4fbaf1 Compare October 8, 2021 06:20
@joeyleeeeeee97
Copy link
Contributor Author

https://ci.adoptopenjdk.net/job/Grinder/1661/console
I run this in Grinder, test passed but I got a

14:49:50  TAP Reports Processing: START
14:49:50  Looking for TAP results report in workspace using pattern: **/*.tap
14:49:51  Did not find any matching files. Setting build result to FAILURE.

Did I miss something?

@smlambert
Copy link
Contributor

re: #457 (comment) I've added a comment on adoptium/aqa-tests#2970 - I think you just need to include the TEST_STATUS in the <command> tag.

@Mesbah-Alam
Copy link
Contributor

Another grinder link testing this PR : https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/1686/console

Copy link
Contributor

@Mesbah-Alam Mesbah-Alam left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

Most October release activities have finished, so we can now merge this and its related aqa-tests PR in, thanks @joeyleeeeeee97 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants