Skip to content

Commit 9401a40

Browse files
amontenegroamontenegro
authored
amontenegro/PD-2487 PD-2486 PD-2483 PD-2485 PD-2484 Oauth fix multiple bugs (#7375)
* redirect_uri is optional on auth code exchange * Redirect uri is not a parameter on the client credentials grant * Fix 500 error when scope is empty * Fix 500 error on missing grant_type * Fix unit tests
1 parent 39f0420 commit 9401a40

File tree

3 files changed

+42
-21
lines changed

3 files changed

+42
-21
lines changed

‎orcid-api-common/src/main/java/org/orcid/api/common/OrcidApiCommonEndpoints.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.orcid.persistence.jpa.entities.OrcidOauth2TokenDetail;
3030
import org.springframework.beans.factory.annotation.Value;
3131
import org.springframework.http.HttpHeaders;
32+
import org.springframework.security.oauth2.common.exceptions.UnsupportedGrantTypeException;
3233
import org.springframework.stereotype.Component;
3334
import org.springframework.web.bind.annotation.RequestParam;
3435

@@ -64,6 +65,10 @@ public Response obtainOauth2TokenPost(@HeaderParam("Authorization") @DefaultValu
6465
throws IOException, URISyntaxException, InterruptedException {
6566

6667
// Token delegation is not implemented in the authorization server
68+
if(grantType == null) {
69+
throw new UnsupportedGrantTypeException("grant_type is missing");
70+
}
71+
6772
if(Features.OAUTH_AUTHORIZATION_CODE_EXCHANGE.isActive() && AuthCodeExchangeForwardUtil.AUTH_SERVER_ALLOWED_GRANT_TYPES.contains(grantType)) {
6873
Response response = null;
6974
switch (grantType) {

‎orcid-web/src/main/java/org/orcid/frontend/web/controllers/OauthGenericCallsController.java‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.springframework.http.HttpHeaders;
3333
import org.springframework.http.HttpStatus;
3434
import org.springframework.http.ResponseEntity;
35+
import org.springframework.security.oauth2.common.exceptions.UnsupportedGrantTypeException;
3536
import org.springframework.stereotype.Controller;
3637
import org.springframework.web.bind.annotation.RequestBody;
3738
import org.springframework.web.bind.annotation.RequestMapping;
@@ -61,6 +62,13 @@ public class OauthGenericCallsController extends OauthControllerBase {
6162
@RequestMapping(value = "/oauth/token", consumes = MediaType.APPLICATION_FORM_URLENCODED, produces = MediaType.APPLICATION_JSON)
6263
public ResponseEntity<?> obtainOauth2TokenPost(HttpServletRequest request) throws IOException, URISyntaxException, InterruptedException {
6364
String grantType = request.getParameter("grant_type");
65+
if(grantType == null) {
66+
OAuthError error = new OAuthError();
67+
error.setErrorDescription("grant_type is missing");
68+
error.setError(OAuthError.UNSUPPORTED_GRANT_TYPE);
69+
error.setResponseStatus(Response.Status.BAD_REQUEST);
70+
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(error);
71+
}
6472
if(Features.OAUTH_AUTHORIZATION_CODE_EXCHANGE.isActive() && AuthCodeExchangeForwardUtil.AUTH_SERVER_ALLOWED_GRANT_TYPES.contains(grantType)) {
6573
String clientId = request.getParameter("client_id");
6674
String clientSecret = request.getParameter("client_secret");
@@ -73,26 +81,30 @@ public ResponseEntity<?> obtainOauth2TokenPost(HttpServletRequest request) throw
7381
String requestedTokenType = request.getParameter("requested_token_type");
7482

7583
Response response = null;
76-
77-
switch (grantType) {
78-
case OrcidOauth2Constants.GRANT_TYPE_AUTHORIZATION_CODE:
79-
response = authCodeExchangeForwardUtil.forwardAuthorizationCodeExchangeRequest(clientId, clientSecret, redirectUri, code);
80-
break;
81-
case OrcidOauth2Constants.GRANT_TYPE_REFRESH_TOKEN:
82-
response = authCodeExchangeForwardUtil.forwardRefreshTokenRequest(clientId, clientSecret, refreshToken, scopeList);
83-
break;
84-
case OrcidOauth2Constants.GRANT_TYPE_CLIENT_CREDENTIALS:
85-
response = authCodeExchangeForwardUtil.forwardClientCredentialsRequest(clientId, clientSecret, scopeList);
86-
break;
87-
case IETF_EXCHANGE_GRANT_TYPE:
88-
response = authCodeExchangeForwardUtil.forwardTokenExchangeRequest(clientId, clientSecret, subjectToken, subjectTokenType, requestedTokenType, scopeList);
89-
break;
84+
try {
85+
switch (grantType) {
86+
case OrcidOauth2Constants.GRANT_TYPE_AUTHORIZATION_CODE:
87+
response = authCodeExchangeForwardUtil.forwardAuthorizationCodeExchangeRequest(clientId, clientSecret, redirectUri, code);
88+
break;
89+
case OrcidOauth2Constants.GRANT_TYPE_REFRESH_TOKEN:
90+
response = authCodeExchangeForwardUtil.forwardRefreshTokenRequest(clientId, clientSecret, refreshToken, scopeList);
91+
break;
92+
case OrcidOauth2Constants.GRANT_TYPE_CLIENT_CREDENTIALS:
93+
response = authCodeExchangeForwardUtil.forwardClientCredentialsRequest(clientId, clientSecret, scopeList);
94+
break;
95+
case IETF_EXCHANGE_GRANT_TYPE:
96+
response = authCodeExchangeForwardUtil.forwardTokenExchangeRequest(clientId, clientSecret, subjectToken, subjectTokenType, requestedTokenType, scopeList);
97+
break;
98+
}
99+
HttpHeaders responseHeaders = new HttpHeaders();
100+
responseHeaders.set(Features.OAUTH_AUTHORIZATION_CODE_EXCHANGE.name(),
101+
"ON");
102+
return ResponseEntity.status(response.getStatus()).headers(responseHeaders).body(response.getEntity());
103+
} catch(Exception e) {
104+
OAuthError error = OAuthErrorUtils.getOAuthError(e);
105+
HttpStatus status = HttpStatus.valueOf(error.getResponseStatus().getStatusCode());
106+
return ResponseEntity.status(status).body(error);
90107
}
91-
92-
HttpHeaders responseHeaders = new HttpHeaders();
93-
responseHeaders.set(Features.OAUTH_AUTHORIZATION_CODE_EXCHANGE.name(),
94-
"ON");
95-
return ResponseEntity.status(response.getStatus()).headers(responseHeaders).body(response.getEntity());
96108
} else {
97109
String authorization = request.getHeader("Authorization");
98110
Enumeration<String> paramNames = request.getParameterNames();

‎orcid-web/src/test/java/org/orcid/frontend/web/controllers/OauthGenericCallsControllerTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ public void setUp() {
4646
public void testObtainOauth2TokenPost() throws IOException, URISyntaxException, InterruptedException {
4747
when(orcidClientCredentialEndPointDelegator.obtainOauth2Token(isNull(), any())).thenReturn(
4848
Response.ok("some-success-entity").build());
49-
ResponseEntity<?> responseEntity = controller.obtainOauth2TokenPost(new MockHttpServletRequest());
49+
MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest();
50+
mockHttpServletRequest.addParameter("grant_type", "client_credentials");
51+
ResponseEntity<?> responseEntity = controller.obtainOauth2TokenPost(mockHttpServletRequest);
5052
assertNotNull(responseEntity);
5153
assertNotNull(responseEntity.getBody());
5254
assertEquals("some-success-entity", responseEntity.getBody());
@@ -56,7 +58,9 @@ public void testObtainOauth2TokenPost() throws IOException, URISyntaxException,
5658
public void testObtainOauth2TokenPostLockedClient() throws IOException, URISyntaxException, InterruptedException {
5759
when(orcidClientCredentialEndPointDelegator.obtainOauth2Token(isNull(), any())).thenThrow(
5860
new LockedException("Client is locked"));
59-
ResponseEntity<?> responseEntity = controller.obtainOauth2TokenPost(new MockHttpServletRequest());
61+
MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest();
62+
mockHttpServletRequest.addParameter("grant_type", "client_credentials");
63+
ResponseEntity<?> responseEntity = controller.obtainOauth2TokenPost(mockHttpServletRequest);
6064
assertNotNull(responseEntity);
6165
assertNotNull(responseEntity.getBody());
6266
assertTrue(responseEntity.getBody() instanceof OAuthError);

0 commit comments

Comments
 (0)