On Tue, 30 Jan 2024 21:58:28 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> This code change adds an alternative implementation of user-based
>> authorization `Subject` APIs that doesn't depend on Security Manager APIs.
>> Depending on if the Security Manager is allowed, the methods store the
>> current subject differently. See the spec change in the `Subject.java` file
>> for details. When the Security Manager APIs are finally removed in a future
>> release, this new implementation will be only implementation for these
>> methods.
>>
>> One major change in the new implementation is that `Subject.getSubject`
>> always throws an `UnsupportedOperationException` since it has an
>> `AccessControlContext` argument but the current subject is no longer
>> associated with an `AccessControlContext` object.
>>
>> Now it's the time to migrate from the `getSubject` and `doAs` methods to
>> `current` and `callAs`. If the user application is simply calling
>> `getSubject(AccessController.getContext())`, then switching to `current()`
>> would work. If the `AccessControlContext` argument is retrieved from an
>> earlier `getContext()` call and the associated subject might be different
>> from that of the current `AccessControlContext`, then instead of storing the
>> previous `AccessControlContext` object and passing it into `getSubject` to
>> get the "previous" subject, the application should store the `current()`
>> return value directly.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fix MBeanServerFileAccessController, more test in SM
src/java.base/share/classes/javax/security/auth/Subject.java line 112:
> 110: *
> 111: * <p><b><a id="sm-allowed">These methods behave differently depending on
> 112: * whether a security manager is allowed or disallowed</a></b>:
Suggest linking "a security manager is allowed or disallowed" to
`SecurityManager.html#set-security-manager`.
src/java.base/share/classes/javax/security/auth/Subject.java line 120:
> 118: * {@code getSubject(AccessControlContext)} method.
> 119: * <li>If a security manager is not allowed, which means it
> 120: * {@linkplain System#setSecurityManager is not set and not allowed to
> be set
I think `SecurityManager.html#set-security-manager` is a better (more
informative) link. Also, not sure why it is linked here but not in the "If a
security manager is allowed" paragraph. If you link it in the first sentence
("These methods behave differently ...) then you probably only need one link
and don't need this link.
test/jdk/javax/security/auth/Subject/CallAsWithScopedValue.java line 27:
> 25: * @test
> 26: * @bug 8296244
> 27: * @enablePreview
Can you add a comment as to why `enablePreview` is needed? (Assume it is for
`StructuredTaskScope`.)
test/jdk/javax/security/auth/Subject/Compat.java line 34:
> 32: /*
> 33: * @test
> 34: * @run main/othervm -Djava.security.manager=allow Compat
Missing `@summary` and `@bug`.
test/jdk/javax/security/auth/Subject/UnsupportedSV.java line 59:
> 57:
> 58: // TODO: Still has no way to reject the following code.
> 59: // Here, AccessController::getContext returns a plan ACC without
typo: s/plan/plain/
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511419716
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511424024
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511380094
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511393254
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511366395
