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

test: Authorization Grant Flow without Redirect URI #2484

Merged
merged 1 commit into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion uaa/src/main/webapp/WEB-INF/spring/oauth-clients.xml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
<entry key="secret" value="loginsecret"/>
<entry key="scope" value="openid,oauth.approvals"/>
<entry key="authorized-grant-types" value="client_credentials,authorization_code"/>
<entry key="redirect-uri" value="http://localhost/**,http://localhost:7000/**"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this change? Is this required for the added test?

Copy link
Member Author

@strehle strehle Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it mainly is the revert of my change https://github.com/cloudfoundry/uaa/pull/2403/files#diff-d02078cfeded6b34f4eb0a68b222758f944db9bbe2fa1227249a9c782718cf09R85

the login client is used in our internal forked scenario and it was broken. Because UAA has this hidden feature of redirect uris. It means if you have only one redirect uri defined you can omit the parameter redirect_uri in authorize request.
If you have an array in the client you cannot use this feature anymore therefore removed the 2nd entry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and create an explicit test for this hidden feature

<entry key="redirect-uri" value="http://localhost/**"/>
<entry key="authorities"
value="oauth.login,scim.write,clients.read,notifications.write,critical_notifications.write,emails.write,scim.userids,password.write,idps.write"/>
<entry key="autoapprove" value="true"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper;
import org.cloudfoundry.identity.uaa.test.TestAccountSetup;
import org.cloudfoundry.identity.uaa.test.UaaTestAccounts;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.springframework.http.HttpEntity;
Expand All @@ -32,6 +33,7 @@
import java.util.Map;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.hamcrest.CoreMatchers.containsString;
Expand Down Expand Up @@ -192,6 +194,34 @@ public void testZoneInactive() {
assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode());
}

@Test
public void testAuthorizationRequestWithoutRedirectUri() {

Map<String, String> body = IntegrationTestUtils.getAuthorizationCodeTokenMap(serverRunning,
testAccounts,
"login",
"loginsecret",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this client has only one redirect uri therefore you can omit it.

testAccounts.getUserName(),
testAccounts.getPassword(),
null,
null,
null,
null,
false);

assertNotNull("Token not received", body.get("access_token"));

try {
IntegrationTestUtils.getAuthorizationCodeTokenMap(serverRunning, testAccounts, "app", "appclientsecret",
testAccounts.getUserName(), testAccounts.getPassword(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the app client has 2 entries for redirect-uri thus here it does not work.

These tests simply should help to know what we have already, nothing new

null, null, null, null, false);
} catch (AssertionError error) {
// expected
return;
}
Assert.fail("Token retrival not allowed");
}

public void testSuccessfulAuthorizationCodeFlow_Internal() {
AuthorizationCodeResourceDetails resource = testAccounts.getDefaultAuthorizationCodeResource();

Expand Down