- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 9.2k
 
[JEP-227] Replace Acegi Security with Spring Security & upgrade Spring Framework #4848
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
Changes from 167 commits
2caf075
              82522cf
              62eba9c
              0c02c44
              d898b35
              f573052
              07412a9
              0613a19
              6af8f1c
              9111057
              6601d98
              b85c0d9
              cfd9831
              085f525
              26060aa
              c6f0e12
              c3f9efd
              a8a661e
              76b0885
              12598a7
              a985a50
              4201baa
              a36f410
              ae31edd
              6434178
              da40e4a
              1ed7887
              3f163d8
              abb439d
              100ca91
              58364c2
              29e1b95
              0ad56da
              a4475c4
              6dad321
              62e73df
              998d673
              aa73f20
              d052fb8
              1f14f3c
              044c42c
              0609eaf
              5354321
              091d0ca
              11ec923
              4bdec83
              d8e6bea
              84cb57b
              cdf2e46
              c95b476
              3910ebf
              8a5bdf2
              7ca54e4
              091bc23
              43e8d29
              2aec3cb
              4c7b6b3
              3a2d857
              736bca4
              9902338
              061e106
              06d4b16
              5adba3e
              a5d60e4
              7fdb8db
              54c65ed
              3094ae5
              30b0079
              b1fcbc6
              6d96228
              f3e6c04
              f69be2c
              ecf3acc
              c382733
              4d7f00d
              abf8251
              7c5ce69
              0ae2aaa
              bd53bea
              034ecff
              b11f3e4
              f32ed4f
              232194d
              530455c
              545e347
              e27ec10
              0a32650
              c9662fb
              19cb4ad
              f49afdf
              09feb27
              b7f1acc
              340e0f9
              d0a125a
              697e555
              ffdf039
              a175d17
              a73186b
              8711c7b
              28ddf31
              d545242
              2f3a61e
              f77173c
              1abe5c6
              6851e32
              4731952
              e90cfa4
              50a470f
              1ebb6a5
              a67b028
              d94651e
              8f71d91
              7a063db
              1e41d5a
              cc029a3
              7b87b21
              b380b78
              64f820e
              b7ed635
              5711c49
              7befcfd
              80bcc26
              a878549
              4845972
              e2b6ff9
              1a90114
              929dcd3
              5f119f5
              2f6380c
              fe269a9
              5d87079
              4eaf883
              17a7fcb
              b7d7d89
              fd744d2
              7fa8a12
              2be2146
              657338c
              4bb45a7
              862e4e5
              2bf6185
              5b60ed0
              c020f8a
              ba608c3
              dc589f1
              1039b2b
              419e44e
              950666e
              14f2074
              df3ca87
              e69a339
              b6d5a6e
              4bed0db
              6548efe
              eb4e22d
              df7c2e9
              83e4fe9
              da3d17b
              7ff3ff5
              7646dfd
              d2ae6ca
              3e3792d
              3ce4ca3
              899773e
              757e1a7
              ffe273a
              a829508
              f04114c
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -385,16 +385,12 @@ THE SOFTWARE. | |
| <artifactId>commons-jexl</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.acegisecurity</groupId> | ||
| <artifactId>acegi-security</artifactId> | ||
| <groupId>org.springframework.security</groupId> | ||
| <artifactId>spring-security-web</artifactId> | ||
| <exclusions> | ||
| <exclusion> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-remoting</artifactId> | ||
| </exclusion> | ||
| <exclusion> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-support</artifactId> | ||
| <artifactId>spring-jcl</artifactId> | ||
| </exclusion> | ||
| </exclusions> | ||
| </dependency> | ||
| 
        
          
        
         | 
    @@ -412,22 +408,6 @@ THE SOFTWARE. | |
| <groupId>org.fusesource.jansi</groupId> | ||
| <artifactId>jansi</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <!-- | ||
| for Grails spring bean builder. | ||
| Ideally we should be able to modify BeanBuilder so as not to depend on this. | ||
| --> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-webmvc</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-core</artifactId> | ||
| </dependency> | ||
| <dependency><!-- Jenkins core doesn't use it but HUDSON-3811 requires us to put it. --> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-aop</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-api</artifactId> | ||
| 
          
            
          
           | 
    @@ -867,5 +847,41 @@ THE SOFTWARE. | |
| <maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile> | ||
| </properties> | ||
| </profile> | ||
| <profile> | ||
| <id>japicmp</id> | ||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>com.github.siom79.japicmp</groupId> | ||
| <artifactId>japicmp-maven-plugin</artifactId> | ||
| <version>0.14.4-20200728.214757-1</version> <!-- TODO https://github.com/siom79/japicmp/pull/266 --> | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly doesn't look like something we want to merge in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is OK because this is only used in a profile we active in CI, so if my PR never gets picked up we can either drop this profile, or use the last mojo release which will work for most purposes (just not complex library replacements like is done here). Happy to remove it if you are uneasy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with leaving it. I actually ran the profile and examined the results. If Daniel is concerned, though, I readily accept the removal.  | 
||
| <configuration> | ||
| <parameter> | ||
| <!-- see https://siom79.github.io/japicmp/MavenPlugin.html --> | ||
| <oldVersionPattern>\d+[.]\d+</oldVersionPattern> | ||
| <!-- <onlyModified>true</onlyModified> --> | ||
| <onlyBinaryIncompatible>true</onlyBinaryIncompatible> | ||
| </parameter> | ||
| <oldClassPathDependencies> | ||
| <dependency> <!-- provided, so not visible in flattened artifact --> | ||
| <groupId>javax.servlet</groupId> | ||
| <artifactId>javax.servlet-api</artifactId> | ||
| <version>3.1.0</version> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| </oldClassPathDependencies> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <phase>verify</phase> | ||
| <goals> | ||
| <goal>cmp</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </profile> | ||
| </profiles> | ||
| </project> | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| package hudson; | ||
| 
     | 
||
| import org.acegisecurity.AcegiSecurityException; | ||
| import org.apache.commons.jelly.JellyContext; | ||
| import org.apache.commons.jelly.JellyException; | ||
| import org.apache.commons.jelly.expression.Expression; | ||
| 
        
          
        
         | 
    @@ -15,6 +14,7 @@ | |
| import java.util.logging.Logger; | ||
| import org.kohsuke.stapler.Stapler; | ||
| import org.kohsuke.stapler.StaplerRequest; | ||
| import org.springframework.security.access.AccessDeniedException; | ||
| 
     | 
||
| /** | ||
| * {@link ExpressionFactory} so that security exception aborts the page rendering. | ||
| 
          
            
          
           | 
    @@ -72,7 +72,7 @@ public Object evaluate(JellyContext context) { | |
| CURRENT_CONTEXT.set(context); | ||
| JexlContext jexlContext = new JellyJexlContext( context ); | ||
| return expression.evaluate(jexlContext); | ||
| } catch (AcegiSecurityException e) { | ||
| } catch (AccessDeniedException e) { | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no replacement for   | 
||
| // let the security exception pass through | ||
| throw e; | ||
| } catch (Exception e) { | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -578,7 +578,7 @@ public <T> void onProvision(ProvisionInvocation<T> provision) { | |
| // so that we invoke them before derived class one. This isn't specified in JSR-250 but implemented | ||
| // this way in Spring and what most developers would expect to happen. | ||
| 
     | 
||
| final Set<Class> interfaces = ClassUtils.getAllInterfacesAsSet(instance); | ||
| final Set<Class<?>> interfaces = ClassUtils.getAllInterfacesAsSet(instance); | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a signature change in Spring.  | 
||
| 
     | 
||
| while (c != Object.class) { | ||
| Arrays.stream(c.getDeclaredMethods()) | ||
| 
          
            
          
           | 
    @@ -607,7 +607,7 @@ public <T> void onProvision(ProvisionInvocation<T> provision) { | |
| * This allows to introspect metadata for a method which is both declared in parent class and in implemented | ||
| * interface(s). {@code interfaces} typically is obtained by {@link ClassUtils#getAllInterfacesAsSet} | ||
| */ | ||
| Collection<Method> getMethodAndInterfaceDeclarations(Method method, Collection<Class> interfaces) { | ||
| Collection<Method> getMethodAndInterfaceDeclarations(Method method, Collection<Class<?>> interfaces) { | ||
| final List<Method> methods = new ArrayList<>(); | ||
| methods.add(method); | ||
| 
     | 
||
| 
          
            
          
           | 
    ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to merge this in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⇒ #4848 (comment)