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

Remove @Configuration meta-annotation from @Enable annotations #6613

Closed
vpavic opened this issue Mar 14, 2019 · 8 comments · Fixed by #11653
Closed

Remove @Configuration meta-annotation from @Enable annotations #6613

vpavic opened this issue Mar 14, 2019 · 8 comments · Fixed by #11653
Assignees
Labels
in: config An issue in spring-security-config status: duplicate A duplicate of another issue type: breaks-passivity A change that breaks passivity with the previous release type: enhancement A general enhancement

Comments

@vpavic
Copy link
Contributor

vpavic commented Mar 14, 2019

Currently, all Spring Security's @Enable annotations are meta-annotated with @Configuration. While convenient, this is not consistent with the rest of the Spring projects and most notably Spring Framework's @Enable annotations.

Additionally, the introduction of support for @Configuration(proxyBeanMethods=false) in Spring Framework provides a compelling reason to remove @Configuration meta-annotation from Spring Security's @Enable annotations and allow users to opt into their preferred configuration mode.

@vpavic vpavic added this to the 6.x milestone Mar 14, 2019
@rwinch rwinch added the status: waiting-for-triage An issue we've not yet triaged label Nov 16, 2021
@eleftherias eleftherias added in: config An issue in spring-security-config type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 10, 2022
@rwinch rwinch added the type: breaks-passivity A change that breaks passivity with the previous release label Jun 13, 2022
@rwinch rwinch self-assigned this Jul 5, 2022
@sjohnr
Copy link
Member

sjohnr commented Jul 26, 2022

Note that we will want to update the documentation examples to include @Configuration in places where it's currently missing.

@jsattler
Copy link
Contributor

@sjohnr I would like to help on this, as I was anyhow looking into the documentation part.

@sjohnr sjohnr assigned jsattler and unassigned rwinch Jul 27, 2022
@sjohnr
Copy link
Member

sjohnr commented Jul 27, 2022

It's yours @jsattler! Keep in mind we'll want to update the reference documentation, javadoc and samples. I'll open a ticket to update the samples, and if you don't feel you can tackle that part just let me know.

@jsattler
Copy link
Contributor

jsattler commented Jul 27, 2022

@sjohnr I will take care of the reference documentation, javadoc and samples. The actual removal of the @Configuration from the @Enable* annotations will be tackled in a separate PR, as this seems to be a breaking change, correct? If it is tackled in a separate PR should I anyhow already update the tests using only @Enable*?

@sjohnr
Copy link
Member

sjohnr commented Jul 29, 2022

@jsattler yes, please do update tests if necessary! I think it can be done under this issue, but if you would like a separate issue let me know. And yes, this is a breaking change so I believe it will only be done in 6.0 (main). @rwinch can correct me if I'm wrong.

jsattler added a commit to jsattler/spring-security that referenced this issue Jul 30, 2022
Before, Spring Security's @enable* annotations were meta-annotated with @configuration.
While convenient, this is not consistent with the rest of the Spring projects and most notably
Spring Framework's @enable annotations. Additionally, the introduction of support for
@configuration(proxyBeanMethods=false) in Spring Framework provides a compelling reason to
remove @configuration meta-annotation from Spring Security's @enable annotations and allow
users to opt into their preferred configuration mode.

Closes spring-projectsgh-6613

Signed-off-by: Joshua Sattler <joshua.sattler@mailbox.org>
jsattler added a commit to jsattler/spring-security that referenced this issue Jul 30, 2022
Before, Spring Security's @enable* annotations were meta-annotated with @configuration.
While convenient, this is not consistent with the rest of the Spring projects and most notably
Spring Framework's @enable annotations. Additionally, the introduction of support for
@configuration(proxyBeanMethods=false) in Spring Framework provides a compelling reason to
remove @configuration meta-annotation from Spring Security's @enable annotations and allow
users to opt into their preferred configuration mode.

Closes spring-projectsgh-6613

Signed-off-by: Joshua Sattler <joshua.sattler@mailbox.org>
@jsattler
Copy link
Contributor

Quite some files changed, hope that's okay for a single PR. If you have any suggestions to split this up, please let me know.

@rwinch
Copy link
Member

rwinch commented Aug 9, 2022

A note for self. I used the following to check for missing @Configuration:

import re
import sys


enable_regex = r".*@Enable[a-zA-Z]+(Security|Authentication).*"
config_regex = r".*@Configuration.*"
A = 2
B = 2

file_name = sys.argv[1]
try:
	file = open(file_name, 'r')
	lines = file.readlines()
except Exception as err:
	print ("Could not open file:", file_name, repr(err))
	sys.exit()


def find_regex_in_range(lines, regex, range):
	for index in range:
		peek_line = lines[index - 1]
		if (re.match(regex, peek_line)):
			return True
	return False

def print_range(lines, range):
	for index in range:
		peek_line = lines[index - 1]
		print(f"{index} {peek_line}", end = "")

line_count = 0

for line in lines:
	line_count += 1
	if (re.match(enable_regex, line)):
		before_range = range(line_count - B, line_count)
		after_range = range(line_count + 1, line_count + A + 1)
		if (find_regex_in_range(lines, config_regex, before_range) or find_regex_in_range(lines, config_regex, after_range)):
			continue
		print (f"{file_name}")
		print ("---")
		print_range(lines, before_range)
		print (f"{line_count} {line}", end = "")
		print_range(lines, after_range)
		print ("---")

Then run

 rg . -l | xargs -I{} python find-enable.py

@rwinch rwinch closed this as completed in 040111a Aug 9, 2022
rwinch added a commit that referenced this issue Aug 9, 2022
@rwinch rwinch moved this to Done in Spring Security Team Aug 9, 2022
@rwinch rwinch removed this from the 6.0.x milestone Aug 9, 2022
@rwinch rwinch added the status: duplicate A duplicate of another issue label Aug 9, 2022
@rwinch
Copy link
Member

rwinch commented Aug 9, 2022

Closing in favor of gh-11653

jzheaux added a commit that referenced this issue Aug 25, 2022
f2c-ci-robot bot pushed a commit to halo-dev/halo that referenced this issue Sep 25, 2022
#### What type of PR is this?

/kind cleanup
/area core
/milestone 2.0

#### What this PR does / why we need it:

This PR mainly upgrades version of dependencies and removes unused dependencies.

See the following references for more:

- https://github.com/spring-projects/spring-boot/releases/tag/v3.0.0-M5
- https://github.com/jhy/jsoup/releases/tag/jsoup-1.15.3
- spring-projects/spring-security#6613

#### Does this PR introduce a user-facing change?

```release-note
None
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: duplicate A duplicate of another issue type: breaks-passivity A change that breaks passivity with the previous release type: enhancement A general enhancement
Projects
Status: Done
5 participants