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

fixes #648: Add the localstorage challenge #716

Merged
merged 30 commits into from
May 5, 2023

Conversation

Novice-expert
Copy link
Contributor

@Novice-expert Novice-expert commented Mar 20, 2023

What kind of changes does this PR include?

  • Fixes or refactors
  • A new challenge
  • Additional documentation
  • Something else

Description

This PR adds a new challenge i.e challenge 29.

Relations

Closes #648

References

Checklist:

  • All the contributions made are solely the work of me and my co-authors
  • I tested the changes in this PR (if applicable)
  • I added unit tests to ensure my change works (when change in Java or on front-end code)
  • The PR passes pre-commit hooks and automated tests

What kind of changes does this PR include?

-   [ ] Fixes or refactors
-   [ ✔] A new challenge
-   [ ] Additional documentation
-   [ ] Something else

Description

This PR adds a new challenge i.e challenge 29.

Relations

Closes  OWASP#648

References

LocalStorage

Checklist:

-   [✔ ] All the contributions made are solely the work of me and my co-authors
-   [ ✔] I tested the changes in this PR (if applicable)
-   [✔ ] I added unit tests to ensure my change works (when change in Java or on front-end code)
-   [✔ ] The PR passes pre-commit hooks and automated tests
Copy link
Collaborator

@commjoen commjoen left a comment

Choose a reason for hiding this comment

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

Hi @Novice-expert ! Thank you very much for your first submission for a Challenge in OWASP WrongSecrets! That's a great first step! Please have a look at the feedback and update the PR :).

separates controller, makes random local storage string
re-added missing swagger annotations
Fixes Challenge 29 codebase:

1: Separated controller and Challenge29 code.

2:Random string generation at every successful run of localhost.
Copy link
Collaborator

@commjoen commjoen left a comment

Choose a reason for hiding this comment

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

Almost there! Can you move the secret key to its own controller class with challenge29 as its member? And can you update the tests to tests that controller as well?

Adds dedicated rest controller for challenge 29
Update index.html endpoint for restcontroller
dedicated Rest Controller class
Copy link
Collaborator

@commjoen commjoen left a comment

Choose a reason for hiding this comment

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

Allrighty! We are almost there! Looks much better than before: minor remarks for now :)

1:- Added hidden annotation so that it doesn't through swapper API generation.

2:- Modified class Challenge29 code as per general and uniform practice.

3:- Consequently, Modified class
Challenge29Test
Now the maven builds successfully.
The ChallengeRestControllerTest works fine!
Adds ctf handler snippet
fixes identation
readds comments in challenge.java
fixes checkstyles
@commjoen commjoen changed the title fixes #648 fixes #648: Add the localstorage challenge Apr 2, 2023
Copy link
Collaborator

@commjoen commjoen left a comment

Choose a reason for hiding this comment

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

This looks to be a good challenge! @bendehaan can you please check the texts?

@commjoen commjoen self-requested a review April 3, 2023 03:16
@commjoen
Copy link
Collaborator

commjoen commented Apr 3, 2023

Pleave have a look at the test failures... something seems to go wrong..

fixes index.html to reduce load time
updates index.html, fixes indentation
Copy link
Collaborator

@commjoen commjoen left a comment

Choose a reason for hiding this comment

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

Thank you! Almost there! @bendehaan can you do a text review please :-)?

commjoen and others added 3 commits May 5, 2023 09:09
…roller.java

Co-authored-by: Ben de Haan <53901866+bendehaan@users.noreply.github.com>
Co-authored-by: Ben de Haan <53901866+bendehaan@users.noreply.github.com>
@commjoen commjoen merged commit 95596fe into OWASP:master May 5, 2023
13 of 14 checks passed
@commjoen
Copy link
Collaborator

commjoen commented May 5, 2023

Thank you @Novice-expert !

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.

New challenge: save a secret in FrontEnd localStorage in browser
3 participants