Skip to content

Feat/hardening #129

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

Feat/hardening #129

wants to merge 9 commits into from

Conversation

saadmar
Copy link
Contributor

@saadmar saadmar commented Dec 13, 2022

No description provided.

@saadmar saadmar force-pushed the feat/hardening branch 2 times, most recently from d117ffc to 7b025de Compare December 13, 2022 12:17
@github-actions
Copy link
Contributor

github-actions bot commented Dec 20, 2022 •
edited
Loading

Test Results

  48 files  +  10    48 suites  +10   1m 3s ⏱️ +19s
283 tests +111  275 ✔️ +105  6 💤 +4  2 ❌ +2 
301 runs  +129  291 ✔️ +121  8 💤 +6  2 ❌ +2 

For more details on these failures, see this check.

Results for commit d36a574. ± Comparison against base commit 807fb57.

This pull request removes 1 and adds 112 tests. Note that renamed tests count towards both. 
org.sdase.commons.spring.boot.web.security.validation.CustomObjectMapperAdviceTest ‑ shouldPreventStartupIfTracingIsEnabled()

org.sdase.commons.spring.boot.web.security.BehindProxyTest ‑ createLinkWithoutForwardedProtoAndHostHeader()
org.sdase.commons.spring.boot.web.security.BehindProxyTest ‑ useForwardedForHeader()
org.sdase.commons.spring.boot.web.security.BehindProxyTest ‑ useForwardedProtoAndHostHeaderToCreateLink()
org.sdase.commons.spring.boot.web.security.BehindProxyTest ‑ useRegularIpWithoutForwardedByHeader()
org.sdase.commons.spring.boot.web.security.CorsOriginsTest ‑ [1] https://allowed.com
org.sdase.commons.spring.boot.web.security.CorsOriginsTest ‑ [1] https://unknown.com
org.sdase.commons.spring.boot.web.security.CorsOriginsTest ‑ [2] https://bar.foo.com
org.sdase.commons.spring.boot.web.security.CorsOriginsTest ‑ [2] https://not-matching.allowed.com
org.sdase.commons.spring.boot.web.security.CorsOriginsTest ‑ [3] https://deep.matching.foo.com
org.sdase.commons.spring.boot.web.security.CorsOriginsTest ‑ [4] https://foo-pr-1.bar.com
…

♻️ This comment has been updated with latest results.

@saadmar saadmar force-pushed the feat/hardening branch 3 times, most recently from 09248cc to 23b044d Compare January 3, 2023 15:41
@JoergSiebahn JoergSiebahn force-pushed the feat/hardening branch 5 times, most recently from d39368f to 1571f52 Compare January 9, 2023 13:45
@JoergSiebahn JoergSiebahn force-pushed the feat/hardening branch 2 times, most recently from 76f4260 to 4cc9a85 Compare January 12, 2023 16:13
@JoergSiebahn JoergSiebahn marked this pull request as ready for review January 12, 2023 16:28
christopher-cudennec
christopher-cudennec reviewed Jan 13, 2023
View reviewed changes
christopher-cudennec
christopher-cudennec reviewed Jan 13, 2023
View reviewed changes
christopher-cudennec
christopher-cudennec reviewed Jan 13, 2023
View reviewed changes
christopher-cudennec
christopher-cudennec reviewed Jan 13, 2023
View reviewed changes
...configure/src/test/java/org/sdase/commons/spring/boot/web/security/test/SecurityTestApp.java Outdated
@SpringBootApplication
@RestController
@EnableSdaRestGuide
@EnableSdaWebSecurity
public class SecurityTestApp {
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to compare the suggested annotations because I expected only one SDA annotation @EnableSdaPlatform.

I went through the documentation but I could not find a nice howto that tells me how I write a service using Spring Boot Commons. Is this the right place to search for the documentation?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should not need any of these annotations due to auto configuration. I'll take a look how to use auto configuration for the security settings.

Yes, that is the right place for documentation. Maybe we should also add a quick start page, where we document the bare minimum of a build.gradle, the application class and the application.properties. But imho that would be another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this information help us creating our own auto config?
https://www.baeldung.com/spring-boot-custom-auto-configuration#creating-a-custom-auto-configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

Imho that article is already outdated. spring.factories has been replaced by the config we are using..

@JoergSiebahn JoergSiebahn force-pushed the feat/hardening branch 7 times, most recently from d7b8a29 to 94d7ea0 Compare January 18, 2023 08:58
@JoergSiebahn JoergSiebahn force-pushed the feat/hardening branch 4 times, most recently from 168ecdb to a732f09 Compare January 18, 2023 17:34
@JoergSiebahn JoergSiebahn force-pushed the feat/hardening branch 2 times, most recently from 69ac972 to 1ae44cf Compare January 18, 2023 17:49
@JoergSiebahn JoergSiebahn force-pushed the feat/hardening branch 7 times, most recently from 9faf064 to 55370e9 Compare January 18, 2023 18:17
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

93.5% 93.5% Coverage
0.0% 0.0% Duplication

@JoergSiebahn
Copy link
Contributor

Closing in favor of many smaller PRs:

@JoergSiebahn JoergSiebahn deleted the feat/hardening branch January 19, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wurstbrot wurstbrot wurstbrot left review comments

@elbiocaetano elbiocaetano elbiocaetano left review comments

@JoergSiebahn JoergSiebahn JoergSiebahn left review comments

@christopher-cudennec christopher-cudennec christopher-cudennec left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@saadmar @JoergSiebahn @wurstbrot @elbiocaetano @christopher-cudennec