Skip to content

Commit

Permalink
Merge pull request #46748 from nextcloud/refactor/core/security-attri…
Browse files Browse the repository at this point in the history
…butes
  • Loading branch information
provokateurin authored Jul 26, 2024
2 parents 41f7fa6 + c57c3c1 commit 40c72d3
Show file tree
Hide file tree
Showing 39 changed files with 225 additions and 270 deletions.
19 changes: 9 additions & 10 deletions core/Controller/AppPasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
use OC\User\Session;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\ApiRoute;
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
use OCP\AppFramework\Http\Attribute\UseSession;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSForbiddenException;
Expand Down Expand Up @@ -45,16 +48,15 @@ public function __construct(
}

/**
* @NoAdminRequired
* @PasswordConfirmationRequired
*
* Create app password
*
* @return DataResponse<Http::STATUS_OK, array{apppassword: string}, array{}>
* @throws OCSForbiddenException Creating app password is not allowed
*
* 200: App password returned
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
#[ApiRoute(verb: 'GET', url: '/getapppassword', root: '/core')]
public function getAppPassword(): DataResponse {
// We do not allow the creation of new tokens if this is an app password
Expand Down Expand Up @@ -98,15 +100,14 @@ public function getAppPassword(): DataResponse {
}

/**
* @NoAdminRequired
*
* Delete app password
*
* @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
* @throws OCSForbiddenException Deleting app password is not allowed
*
* 200: App password deleted successfully
*/
#[NoAdminRequired]
#[ApiRoute(verb: 'DELETE', url: '/apppassword', root: '/core')]
public function deleteAppPassword(): DataResponse {
if (!$this->session->exists('app_password')) {
Expand All @@ -126,15 +127,14 @@ public function deleteAppPassword(): DataResponse {
}

/**
* @NoAdminRequired
*
* Rotate app password
*
* @return DataResponse<Http::STATUS_OK, array{apppassword: string}, array{}>
* @throws OCSForbiddenException Rotating app password is not allowed
*
* 200: App password returned
*/
#[NoAdminRequired]
#[ApiRoute(verb: 'POST', url: '/apppassword/rotate', root: '/core')]
public function rotateAppPassword(): DataResponse {
if (!$this->session->exists('app_password')) {
Expand All @@ -160,16 +160,15 @@ public function rotateAppPassword(): DataResponse {
/**
* Confirm the user password
*
* @NoAdminRequired
* @BruteForceProtection(action=sudo)
*
* @param string $password The password of the user
*
* @return DataResponse<Http::STATUS_OK, array{lastLogin: int}, array{}>|DataResponse<Http::STATUS_FORBIDDEN, array<empty>, array{}>
*
* 200: Password confirmation succeeded
* 403: Password confirmation failed
*/
#[NoAdminRequired]
#[BruteForceProtection('sudo')]
#[UseSession]
#[ApiRoute(verb: 'PUT', url: '/apppassword/confirm', root: '/core')]
public function confirmUserPassword(string $password): DataResponse {
Expand Down
4 changes: 2 additions & 2 deletions core/Controller/AutoCompleteController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OC\Core\ResponseDefinitions;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\ApiRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCSController;
use OCP\Collaboration\AutoComplete\AutoCompleteEvent;
Expand All @@ -36,8 +37,6 @@ public function __construct(
}

/**
* @NoAdminRequired
*
* Autocomplete a query
*
* @param string $search Text to search for
Expand All @@ -51,6 +50,7 @@ public function __construct(
*
* 200: Autocomplete results returned
*/
#[NoAdminRequired]
#[ApiRoute(verb: 'GET', url: '/autocomplete/get', root: '/core')]
public function get(string $search, ?string $itemType, ?string $itemId, ?string $sorter = null, array $shareTypes = [IShare::TYPE_USER], int $limit = 10): DataResponse {
// if enumeration/user listings are disabled, we'll receive an empty
Expand Down
28 changes: 11 additions & 17 deletions core/Controller/AvatarController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
Expand Down Expand Up @@ -47,10 +50,7 @@ public function __construct(
}

/**
* @NoAdminRequired
* @NoCSRFRequired
* @NoSameSiteCookieRequired
* @PublicPage
*
* Get the dark avatar
*
Expand All @@ -63,6 +63,8 @@ public function __construct(
* 201: Avatar returned
* 404: Avatar not found
*/
#[NoCSRFRequired]
#[PublicPage]
#[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}/dark')]
public function getAvatarDark(string $userId, int $size, bool $guestFallback = false) {
if ($size <= 64) {
Expand Down Expand Up @@ -99,10 +101,7 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f


/**
* @NoAdminRequired
* @NoCSRFRequired
* @NoSameSiteCookieRequired
* @PublicPage
*
* Get the avatar
*
Expand All @@ -115,6 +114,8 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f
* 201: Avatar returned
* 404: Avatar not found
*/
#[NoCSRFRequired]
#[PublicPage]
#[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}')]
public function getAvatar(string $userId, int $size, bool $guestFallback = false) {
if ($size <= 64) {
Expand Down Expand Up @@ -149,9 +150,7 @@ public function getAvatar(string $userId, int $size, bool $guestFallback = false
return $response;
}

/**
* @NoAdminRequired
*/
#[NoAdminRequired]
#[FrontpageRoute(verb: 'POST', url: '/avatar/')]
public function postAvatar(?string $path = null): JSONResponse {
$files = $this->request->getUploadedFile('files');
Expand Down Expand Up @@ -271,9 +270,7 @@ public function postAvatar(?string $path = null): JSONResponse {
}
}

/**
* @NoAdminRequired
*/
#[NoAdminRequired]
#[FrontpageRoute(verb: 'DELETE', url: '/avatar/')]
public function deleteAvatar(): JSONResponse {
try {
Expand All @@ -287,10 +284,9 @@ public function deleteAvatar(): JSONResponse {
}

/**
* @NoAdminRequired
*
* @return JSONResponse|DataDisplayResponse
*/
#[NoAdminRequired]
#[FrontpageRoute(verb: 'GET', url: '/avatar/tmp')]
public function getTmpAvatar() {
$tmpAvatar = $this->cache->get('tmpAvatar');
Expand All @@ -315,9 +311,7 @@ public function getTmpAvatar() {
return $resp;
}

/**
* @NoAdminRequired
*/
#[NoAdminRequired]
#[FrontpageRoute(verb: 'POST', url: '/avatar/cropped')]
public function postCroppedAvatar(?array $crop = null): JSONResponse {
if (is_null($crop)) {
Expand Down
8 changes: 4 additions & 4 deletions core/Controller/CSRFTokenController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\JSONResponse;
use OCP\IRequest;

Expand All @@ -27,15 +29,13 @@ public function __construct(
/**
* Returns a new CSRF token.
*
* @NoAdminRequired
* @NoCSRFRequired
* @PublicPage
*
* @return JSONResponse<Http::STATUS_OK, array{token: string}, array{}>|JSONResponse<Http::STATUS_FORBIDDEN, array<empty>, array{}>
*
* 200: CSRF token returned
* 403: Strict cookie check failed
*/
#[PublicPage]
#[NoCSRFRequired]
#[FrontpageRoute(verb: 'GET', url: '/csrftoken')]
public function index(): JSONResponse {
if (!$this->request->passesStrictCookieCheck()) {
Expand Down
20 changes: 9 additions & 11 deletions core/Controller/ClientFlowLoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\Attribute\UseSession;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Http\StandaloneTemplateResponse;
Expand Down Expand Up @@ -82,10 +85,8 @@ private function stateTokenForbiddenResponse(): StandaloneTemplateResponse {
return $response;
}

/**
* @PublicPage
* @NoCSRFRequired
*/
#[PublicPage]
#[NoCSRFRequired]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/flow')]
public function showAuthPickerPage(string $clientIdentifier = '', string $user = '', int $direct = 0): StandaloneTemplateResponse {
Expand Down Expand Up @@ -150,10 +151,10 @@ public function showAuthPickerPage(string $clientIdentifier = '', string $user =
}

/**
* @NoAdminRequired
* @NoCSRFRequired
* @NoSameSiteCookieRequired
*/
#[NoAdminRequired]
#[NoCSRFRequired]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/flow/grant')]
public function grantPage(string $stateToken = '',
Expand Down Expand Up @@ -203,10 +204,9 @@ public function grantPage(string $stateToken = '',
}

/**
* @NoAdminRequired
*
* @return Http\RedirectResponse|Response
*/
#[NoAdminRequired]
#[UseSession]
#[FrontpageRoute(verb: 'POST', url: '/login/flow')]
public function generateAppPassword(string $stateToken,
Expand Down Expand Up @@ -297,9 +297,7 @@ public function generateAppPassword(string $stateToken,
return new Http\RedirectResponse($redirectUri);
}

/**
* @PublicPage
*/
#[PublicPage]
#[FrontpageRoute(verb: 'POST', url: '/login/flow/apptoken')]
public function apptokenRedirect(string $stateToken, string $user, string $password): Response {
if (!$this->isValidToken($stateToken)) {
Expand Down
37 changes: 15 additions & 22 deletions core/Controller/ClientFlowLoginV2Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\Attribute\UseSession;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\RedirectResponse;
Expand Down Expand Up @@ -55,9 +58,6 @@ public function __construct(
}

/**
* @NoCSRFRequired
* @PublicPage
*
* Poll the login flow credentials
*
* @param string $token Token of the flow
Expand All @@ -66,6 +66,8 @@ public function __construct(
* 200: Login flow credentials returned
* 404: Login flow not found or completed
*/
#[NoCSRFRequired]
#[PublicPage]
#[FrontpageRoute(verb: 'POST', url: '/login/v2/poll')]
public function poll(string $token): JSONResponse {
try {
Expand All @@ -77,10 +79,8 @@ public function poll(string $token): JSONResponse {
return new JSONResponse($creds->jsonSerialize());
}

/**
* @NoCSRFRequired
* @PublicPage
*/
#[NoCSRFRequired]
#[PublicPage]
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/v2/flow/{token}')]
Expand All @@ -96,10 +96,8 @@ public function landing(string $token, $user = ''): Response {
);
}

/**
* @NoCSRFRequired
* @PublicPage
*/
#[NoCSRFRequired]
#[PublicPage]
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/v2/flow')]
Expand Down Expand Up @@ -131,10 +129,10 @@ public function showAuthPickerPage($user = ''): StandaloneTemplateResponse {
}

/**
* @NoAdminRequired
* @NoCSRFRequired
* @NoSameSiteCookieRequired
*/
#[NoAdminRequired]
#[NoCSRFRequired]
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/v2/grant')]
Expand Down Expand Up @@ -170,9 +168,7 @@ public function grantPage(?string $stateToken): StandaloneTemplateResponse {
);
}

/**
* @PublicPage
*/
#[PublicPage]
#[FrontpageRoute(verb: 'POST', url: '/login/v2/apptoken')]
public function apptokenRedirect(?string $stateToken, string $user, string $password) {
if ($stateToken === null) {
Expand Down Expand Up @@ -217,9 +213,7 @@ public function apptokenRedirect(?string $stateToken, string $user, string $pass
return $this->handleFlowDone($result);
}

/**
* @NoAdminRequired
*/
#[NoAdminRequired]
#[UseSession]
#[FrontpageRoute(verb: 'POST', url: '/login/v2/grant')]
public function generateAppPassword(?string $stateToken): Response {
Expand Down Expand Up @@ -270,15 +264,14 @@ private function handleFlowDone(bool $result): StandaloneTemplateResponse {
}

/**
* @NoCSRFRequired
* @PublicPage
*
* Init a login flow
*
* @return JSONResponse<Http::STATUS_OK, CoreLoginFlowV2, array{}>
*
* 200: Login flow init returned
*/
#[NoCSRFRequired]
#[PublicPage]
#[FrontpageRoute(verb: 'POST', url: '/login/v2')]
public function init(): JSONResponse {
// Get client user agent
Expand Down
Loading

0 comments on commit 40c72d3

Please sign in to comment.