Commit 233d87f
Fix HTTP 500 on anonymous CLARIN link rels (#1431)
Three CLARIN link rels answered HTTP 500 instead of 401 for an anonymous
caller on an id that exists:
core/clarinuserregistrations/1/userMetadata 500 (parent: 401)
core/clarinuserregistrations/1/clarinLicenses 500 (parent: 401)
core/clarinlruallowances/242/resourceMapping 500 (parent: 401)
The same rels answer 404 on an id that does not exist, so the route
resolves and the repository body runs - the 500 is thrown inside it.
Two independent causes, both in fork code:
1. ClarinUserRegistrationServiceImpl.authorizeClarinUserRegistrationAction()
dereferenced context.getCurrentUser() with no null check. isAdmin()
returns false rather than throwing for an anonymous context, so the
authorization helper itself threw NullPointerException before it could
throw AuthorizeException. Its sibling authorizeClruaAction() already
carries the Objects.nonNull(currentUser) guard, which is why the
clarinlruallowances rels answer 401 and these two did not.
2. CLRUAResourceMappingLinkRepository let the checked AuthorizeException
escape. RestResourceController.findRelInternal re-throws a
RuntimeException target as-is but wraps a checked one in
new RuntimeException(e), which surfaces as 500. Its two siblings
convert that exception, which is why the same service call produced
401, 401 and 500 from three repositories.
Both defects predate the DSpace 9 upgrade; registering the link
repositories under the plural model name (#1404) only made the bodies
reachable. RestResourceController is byte-identical with dspace-9.3 and
vanilla link repositories use @PreAuthorize plus AccessDeniedException,
so nothing here belongs upstream.
The three link repositories now follow that vanilla pattern: a coarse
@PreAuthorize("hasAuthority('AUTHENTICATED')") matching the parent
repository's findOne, and a translation of the service's checked
AuthorizeException into AccessDeniedException. AccessDeniedException
rather than the fork's RESTAuthorizationException because
Utils.embedRelFromRepository swallows only the former; the Angular UI
embeds userMetadata via followLink(), and any other RuntimeException
raised while embedding would fail the parent request with a 500.
ClarinLinkRestRepositoryBeanNameIT grows from 3 to 10 tests: the three
existing bean-name tests, the fork's one rel on a vanilla model, an
automatic check that every registered LinkRestRepository bean names a
model repository that exists (so a future model that forgets PLURAL_NAME
fails without editing this class), and the anonymous/owner/other-user/
admin matrix for the four rels above.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 11da09b commit 233d87f
5 files changed
Lines changed: 327 additions & 30 deletions
File tree
- dspace-api/src/main/java/org/dspace/content/clarin
- dspace-server-webapp/src
- main/java/org/dspace/app/rest/repository
- test/java/org/dspace/app/rest
Lines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
148 | | - | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
149 | 151 | | |
150 | 152 | | |
151 | 153 | | |
| |||
Lines changed: 17 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
26 | 28 | | |
27 | 29 | | |
28 | 30 | | |
| |||
36 | 38 | | |
37 | 39 | | |
38 | 40 | | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
39 | 48 | | |
40 | 49 | | |
41 | 50 | | |
42 | 51 | | |
43 | | - | |
| 52 | + | |
44 | 53 | | |
45 | 54 | | |
46 | | - | |
47 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
48 | 62 | | |
49 | 63 | | |
50 | 64 | | |
| |||
Lines changed: 16 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
38 | 46 | | |
39 | 47 | | |
40 | 48 | | |
41 | | - | |
| 49 | + | |
42 | 50 | | |
43 | | - | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
44 | 58 | | |
45 | 59 | | |
46 | 60 | | |
| |||
Lines changed: 20 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
26 | 28 | | |
27 | 29 | | |
28 | 30 | | |
| |||
33 | 35 | | |
34 | 36 | | |
35 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
36 | 47 | | |
37 | 48 | | |
38 | 49 | | |
39 | | - | |
| 50 | + | |
40 | 51 | | |
41 | 52 | | |
42 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
43 | 60 | | |
44 | | - | |
| 61 | + | |
45 | 62 | | |
46 | 63 | | |
47 | 64 | | |
| |||
0 commit comments