Skip to content

Commit

Permalink
Merge pull request #693 from DiamondLightSource/improvement/lims-1120…
Browse files Browse the repository at this point in the history
…/unrecognised-message

[LIMS-1120] Display message when user is not in database
  • Loading branch information
gfrn authored Nov 23, 2023
2 parents ff3e520 + cf88a33 commit 9d2d739
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
8 changes: 8 additions & 0 deletions api/src/Controllers/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,14 @@ function authenticateByCode()
$code = $this->app->request()->post('code');
$fedid = $this->authenticateByType()->authenticateByCode($code);
if ($fedid) {
/*
* Since the returned username might not be in the database, given it's returned by
* the SSO provider and not our internal authentication logic, we need to double check
* if it's valid
*/
if (!$this->dataLayer->isUserLoggedIn($fedid)) {
$this->returnError(403, 'User not recognised');
}
$this->returnResponse(200, $this->generateJwtToken($fedid));
} else {
$this->returnError(401, 'Invalid Credentials');
Expand Down
17 changes: 17 additions & 0 deletions api/tests/Controllers/AuthenticationControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ public function testCodeAuthenticationWhenGetValidFedIdReturnsSuccess(): void
$expectedFedID = "FedId012";
$_SERVER['HTTP_HOST'] = "host";

$this->dataLayerStub->method("isUserLoggedIn")->willReturn(true);

$this->setupSlimStubsRequestAndResponse(1, 1);
$authService = $this->setUpAuthControllerWithMockedAuthType("authenticateByCode", $expectedFedID);

Expand Down Expand Up @@ -214,4 +216,19 @@ public function testValidateAuthorisationRedirect(): void
$this->assertContains('Content-Type: application/json', Output::$headers);
$this->assertContains('X-PHP-Response-Code: 302', Output::$headers);
}

public function testReturnForbiddenIfUserIsNotRecognised(): void
{
$this->setupSlimStubsRequestAndResponse(1, 1);

$this->dataLayerStub->method("isUserLoggedIn")->willReturn(false);
$authService = $this->setUpAuthControllerWithMockedAuthType("authenticateByCode", "invalidFedId");

$this->expectExceptionOn( function () use ($authService){
$authService->authenticateByCode();
});

$this->assertContains('Content-Type: application/json', Output::$headers);
$this->assertContains('X-PHP-Response-Code: 403', Output::$headers);
}
}
20 changes: 18 additions & 2 deletions client/src/js/app/views/login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
<hero-title v-show="!skipHome" />

<h1>Login</h1>
<p v-if="sso" class="tw-text-center tw-text-lg">Redirecting to CAS single sign on...</p>
<p v-if="sso && !authError" class="tw-text-center tw-text-lg">Redirecting to CAS single sign on...</p>
<p v-if="authError === 'not-recognised'" class="tw-text-center tw-text-lg">
<b>User not recognised</b>. If you have logged in with your email address,
<a :href="logoutUrl" target="_blank">log out of the SSO provider</a> and <a href="/login">log in</a> again with
your FedID.
</p>

<!-- Wrap the form in an observer component so we can check validation state on submission -->
<validation-observer ref="observer" v-if="!sso" v-slot="{ invalid }">
Expand Down Expand Up @@ -101,6 +106,7 @@ export default {
username: "",
password: "",
redirectUrl: "/current",
authError: null,
};
},
Expand All @@ -120,6 +126,9 @@ export default {
apiUrl: function () {
return this.$store.getters.apiUrl;
},
logoutUrl: function () {
return `https://${this.$store.getters.sso_url}/logout`;
},
},
watch: {
Expand Down Expand Up @@ -186,7 +195,14 @@ export default {
});
} else {
const token = location.href.substring(tokenLocation + ("code".length + 1));
this.$store.dispatch("auth/getToken", token).then(() => this.$router.push(actualRedirectUrl));
this.$store
.dispatch("auth/getToken", token)
.then(() => this.$router.push(actualRedirectUrl))
.catch((e) => {
if (e === "Forbidden") {
this.authError = "not-recognised";
}
});
}
}
},
Expand Down

0 comments on commit 9d2d739

Please sign in to comment.