1
0
mirror of https://github.com/bpatrik/pigallery2.git synced 2025-01-08 04:03:48 +02:00

Fix sharing password. fixes #744

This commit is contained in:
Patrik J. Braun 2023-12-01 19:33:39 +01:00
parent 16bf756582
commit ba9b5292e1
14 changed files with 336 additions and 245 deletions

View File

@ -1,5 +1,5 @@
import {NextFunction, Request, Response} from 'express';
import {CreateSharingDTO, SharingDTO} from '../../common/entities/SharingDTO';
import {CreateSharingDTO, SharingDTO, SharingDTOKey} from '../../common/entities/SharingDTO';
import {ObjectManagers} from '../model/ObjectManagers';
import {ErrorCodes, ErrorDTO} from '../../common/entities/Error';
import {Config} from '../../common/config/private/Config';
@ -33,6 +33,31 @@ export class SharingMWs {
}
}
public static async getSharingKey(
req: Request,
res: Response,
next: NextFunction
): Promise<void> {
if (Config.Sharing.enabled === false) {
return next();
}
const sharingKey = req.params[QueryParams.gallery.sharingKey_params];
try {
req.resultPipe =
{sharingKey: (await ObjectManagers.getInstance().SharingManager.findOne(sharingKey)).sharingKey} as SharingDTOKey;
return next();
} catch (err) {
return next(
new ErrorDTO(
ErrorCodes.GENERAL_ERROR,
'Error during retrieving sharing key',
err
)
);
}
}
public static async createSharing(
req: Request,
res: Response,
@ -50,6 +75,14 @@ export class SharingMWs {
);
}
const createSharing: CreateSharingDTO = req.body.createSharing;
if (Config.Sharing.passwordRequired && !createSharing.password) {
return next(
new ErrorDTO(ErrorCodes.INPUT_ERROR, 'Password is required.')
);
}
let sharingKey = SharingMWs.generateKey();
// create one not yet used
@ -138,6 +171,7 @@ export class SharingMWs {
sharing,
forceUpdate
);
console.log(req.resultPipe);
return next();
} catch (err) {
return next(

View File

@ -156,8 +156,8 @@ export class AuthenticationMWs {
if (
!sharing ||
sharing.expires < Date.now() ||
(Config.Sharing.passwordProtected === true &&
sharing.password &&
((Config.Sharing.passwordRequired === true ||
sharing.password) &&
!PasswordHelper.comparePassword(password, sharing.password))
) {
Logger.warn(LOG_TAG, 'Failed login with sharing:' + sharing.sharingKey + ', bad password');
@ -265,8 +265,9 @@ export class AuthenticationMWs {
return null;
}
// no 'free login' if passwords are required, or it is set
if (
Config.Sharing.passwordProtected === true &&
Config.Sharing.passwordRequired === true ||
sharing.password
) {
return null;

View File

@ -11,6 +11,7 @@ export class SharingRouter {
public static route(app: express.Express): void {
this.addShareLogin(app);
this.addGetSharing(app);
this.addGetSharingKey(app);
this.addCreateSharing(app);
this.addUpdateSharing(app);
this.addListSharing(app);
@ -28,6 +29,21 @@ export class SharingRouter {
);
}
/**
* Used to check the key validity
* @param app
* @private
*/
private static addGetSharingKey(app: express.Express): void {
app.get(
Config.Server.apiPath + '/share/:' + QueryParams.gallery.sharingKey_params + '/key',
// its a public path
SharingMWs.getSharingKey,
ServerTimingMWs.addServerTiming,
RenderingMWs.renderSharing
);
}
private static addGetSharing(app: express.Express): void {
app.get(
Config.Server.apiPath + '/share/:' + QueryParams.gallery.sharingKey_params,

View File

@ -273,12 +273,12 @@ export class ClientSharingConfig {
@ConfigProperty({
tags:
{
name: $localize`Password protected`,
name: $localize`Require password`,
priority: ConfigPriority.advanced
},
description: $localize`Enables password protected sharing links.`,
description: $localize`Requires password protected sharing links.`,
})
passwordProtected: boolean = true;
passwordRequired: boolean = false;
}
@SubConfigClass({tags: {client: true}, softReadonly: true})

View File

@ -1,6 +1,10 @@
import {UserDTO} from './UserDTO';
export interface SharingDTO {
export interface SharingDTOKey {
sharingKey: string;
}
export interface SharingDTO extends SharingDTOKey {
id: number;
path: string;
sharingKey: string;

View File

@ -65,16 +65,16 @@
<li class="nav-item dropdown">
<div class="btn-group" dropdown #dropdown="bs-dropdown" placement="bottom"
[autoClose]="false" container="body">
<button id="button-alignment" dropdownToggle
<button id="button-frame-menu" dropdownToggle
type="button" class="btn btn-tertiary dropdown-toggle"
aria-controls="dropdown-alignment">
aria-controls="dropdown-frame-menu">
<ng-icon class="align-text-top" size="1.2em" name="ionMenuOutline"></ng-icon>
<span *ngIf="isAdmin() && notificationService.numberOfNotifications>0"
class="navbar-badge badge text-bg-warning">{{notificationService.numberOfNotifications}}</span>
</button>
<ul id="dropdown-alignment" *dropdownMenu
<ul id="dropdown-frame-menu" *dropdownMenu
class="dropdown-menu dropdown-menu-right"
role="menu" aria-labelledby="button-alignment">
role="menu" aria-labelledby="button-frame-menu">
<li role="menuitem" class="d-xl-none">
<div style="white-space: nowrap;" class="dropdown-item">

View File

@ -118,12 +118,7 @@ export class GalleryComponent implements OnInit, OnDestroy {
async ngOnInit(): Promise<boolean> {
await this.shareService.wait();
if (
!this.authService.isAuthenticated() &&
(!this.shareService.isSharing() ||
(this.shareService.isSharing() &&
Config.Sharing.passwordProtected === true))
) {
if (!this.authService.isAuthenticated()) {
return this.navigation.toLogin();
}
this.showSearchBar = this.authService.canSearch();

View File

@ -1,11 +1,11 @@
import {Injectable} from '@angular/core';
import {NetworkService} from '../../model/network/network.service';
import {CreateSharingDTO, SharingDTO,} from '../../../../common/entities/SharingDTO';
import {CreateSharingDTO, SharingDTO, SharingDTOKey,} from '../../../../common/entities/SharingDTO';
import {Router, RoutesRecognized} from '@angular/router';
import {BehaviorSubject} from 'rxjs';
import {distinctUntilChanged, filter} from 'rxjs/operators';
import {QueryParams} from '../../../../common/QueryParams';
import {UserDTO} from '../../../../common/entities/UserDTO';
import {UserDTO, UserRoles} from '../../../../common/entities/UserDTO';
import {Utils} from '../../../../common/Utils';
import {Config} from '../../../../common/config/public/Config';
@ -23,6 +23,9 @@ export class ShareService {
public sharingSubject: BehaviorSubject<SharingDTO> = new BehaviorSubject(
null
);
public sharingIsValid: BehaviorSubject<boolean> = new BehaviorSubject(
null
);
public currentSharing = this.sharingSubject
.asObservable()
.pipe(filter((s) => s !== null))
@ -52,7 +55,7 @@ export class ShareService {
const changed = this.sharingKey !== (this.param || this.queryParam);
if (changed) {
this.sharingKey = this.param || this.queryParam || this.sharingKey;
await this.getSharing();
await this.checkSharing();
}
if (this.resolve) {
this.resolve();
@ -69,12 +72,17 @@ export class ShareService {
onNewUser = async (user: UserDTO) => {
if (user && !!user.usedSharingKey) {
// if this is a sharing user or a logged-in user, get sharing key
if (user?.usedSharingKey || user?.role > UserRoles.LimitedGuest) {
if (
user.usedSharingKey !== this.sharingKey ||
(user?.usedSharingKey &&
user?.usedSharingKey !== this.sharingKey) ||
this.sharingSubject.value == null
) {
this.sharingKey = user.usedSharingKey;
this.sharingKey = user.usedSharingKey || this.getSharingKey();
if(!this.sharingKey){ //no key to fetch
return
}
await this.getSharing();
}
if (this.resolve) {
@ -95,12 +103,14 @@ export class ShareService {
public createSharing(
dir: string,
includeSubFolders: boolean,
password: string,
valid: number
): Promise<SharingDTO> {
return this.networkService.postJson('/share/' + dir, {
createSharing: {
includeSubfolders: includeSubFolders,
valid,
...(!!password && {password: password}) // only add password if present
} as CreateSharingDTO,
});
}
@ -143,6 +153,19 @@ export class ShareService {
}
}
private async checkSharing(): Promise<void> {
try {
this.sharingIsValid.next(null);
const sharing = await this.networkService.getJson<SharingDTOKey>(
'/share/' + this.getSharingKey() + '/key'
);
this.sharingIsValid.next(sharing.sharingKey === this.getSharingKey());
} catch (e) {
this.sharingIsValid.next(false);
console.error(e);
}
}
public async getSharingListForDir(
dir: string
): Promise<SharingDTO[]> {

View File

@ -82,11 +82,11 @@
</div>
</div>
<div class="row" *ngIf="passwordProtection">
<div class="row">
<div class="col-4">
<label class="control-label" for="share-password">
<ng-container i18n>Password</ng-container><!--
-->*:
<ng-container i18n>Password</ng-container>
<ng-container *ngIf="passwordRequired">*</ng-container>
</label>
</div>
<div class="col-8">
@ -98,7 +98,7 @@
[(ngModel)]="input.password"
i18n-placeholder
placeholder="Password"
required>
[required]="passwordRequired">
</div>
</div>

View File

@ -36,7 +36,7 @@ export class GalleryShareComponent implements OnInit, OnDestroy {
currentDir = '';
sharing: SharingDTO = null;
contentSubscription: Subscription = null;
readonly passwordProtection = Config.Sharing.passwordProtected;
readonly passwordRequired = Config.Sharing.passwordRequired;
readonly ValidityTypes = ValidityTypes;
modalRef: BsModalRef;
@ -138,11 +138,16 @@ export class GalleryShareComponent implements OnInit, OnDestroy {
}
async get(): Promise<void> {
if(Config.Sharing.passwordRequired && !this.input.password){
this.url = $localize`Set password.`;
return;
}
this.urlValid = false;
this.url = $localize`loading..`;
this.sharing = await this.sharingService.createSharing(
this.currentDir,
this.input.includeSubfolders,
this.input.password,
this.calcValidity()
);
this.url = this.sharingService.getUrl(this.sharing);

View File

@ -7,10 +7,10 @@
</div>
<div class="row card align-self-center">
<div class="card-body">
<div *ngIf="(shareService.currentSharing | async) == shareService.UnknownSharingKey"
<div *ngIf="!(shareService.sharingIsValid | async)"
class="h3 text-center text-danger" i18n>Unknown sharing key.
</div>
<form *ngIf="(shareService.currentSharing | async) != shareService.UnknownSharingKey"
<form *ngIf="(shareService.sharingIsValid | async)"
name="form" id="form" class="form-horizontal" #LoginForm="ngForm" (submit)="onLogin()">
<div class="error-message" [hidden]="loginError==false" i18n>Wrong password</div>
@ -32,6 +32,7 @@
<div class="col-sm-12 controls d-grid gap-2">
<button class="btn btn-primary btn-lg"
id="button-share-login"
[disabled]="!LoginForm.form.valid || inProgress"
type="submit"
name="action" i18n>Enter

View File

@ -75,8 +75,15 @@ describe('PublicRouter', () => {
.get('/share/' + sharingKey);
};
it('should not get default user with passworded share share without password', async () => {
Config.Sharing.passwordProtected = true;
it('should not get default user with passworded share without required password', async () => {
Config.Sharing.passwordRequired = false;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const res = await fistLoad(server, sharing.sharingKey);
shouldHaveInjectedUser(res, null);
});
it('should not get default user with passworded share share with required password', async () => {
Config.Sharing.passwordRequired = true;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const res = await fistLoad(server, sharing.sharingKey);
shouldHaveInjectedUser(res, null);
@ -84,25 +91,13 @@ describe('PublicRouter', () => {
it('should get default user with no-password share', async () => {
Config.Sharing.passwordProtected = true;
Config.Sharing.passwordRequired = false;
const sharing = await RouteTestingHelper.createSharing(testUser);
const res = await fistLoad(server, sharing.sharingKey);
shouldHaveInjectedUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
it('should get default user for no-password share when password protection disabled', async () => {
Config.Sharing.passwordProtected = false;
const sharing = await RouteTestingHelper.createSharing(testUser);
const res = await fistLoad(server, sharing.sharingKey);
shouldHaveInjectedUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
it('should get default user for passworded share when password protection disabled', async () => {
Config.Sharing.passwordProtected = false;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const res = await fistLoad(server, sharing.sharingKey);
shouldHaveInjectedUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
});

View File

@ -74,15 +74,32 @@ describe('SharingRouter', () => {
beforeEach(setUp);
afterEach(tearDown);
it('should login with passworded share', async () => {
Config.Sharing.passwordProtected = true;
it('should login with passworded share when password required', async () => {
Config.Sharing.passwordRequired = true;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const res = await shareLogin(server, sharing.sharingKey, sharing.password);
shouldBeValidUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
it('should login with passworded share when password not required', async () => {
Config.Sharing.passwordRequired = false;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const res = await shareLogin(server, sharing.sharingKey, sharing.password);
shouldBeValidUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
it('should login without passworded share when password not required', async () => {
Config.Sharing.passwordRequired = false;
const sharing = await RouteTestingHelper.createSharing(testUser );
const res = await shareLogin(server, sharing.sharingKey, sharing.password);
shouldBeValidUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
it('should not login with passworded share without password', async () => {
Config.Sharing.passwordProtected = true;
Config.Sharing.passwordRequired = true;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const result = await shareLogin(server, sharing.sharingKey);
@ -92,22 +109,22 @@ describe('SharingRouter', () => {
should.equal(result.body.error.code, ErrorCodes.CREDENTIAL_NOT_FOUND);
});
it('should not login with passworded share but password protection disabled', async () => {
Config.Sharing.passwordProtected = false;
const sharing = await RouteTestingHelper.createSharing(testUser, 'secret_pass');
const res = await shareLogin(server, sharing.sharingKey);
shouldBeValidUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
it('should login with no-password share', async () => {
Config.Sharing.passwordProtected = true;
it('should not login to share without password when password required', async () => {
Config.Sharing.passwordRequired = false;
const sharing = await RouteTestingHelper.createSharing(testUser);
const res = await shareLogin(server, sharing.sharingKey, sharing.password);
shouldBeValidUser(res, RouteTestingHelper.getExpectedSharingUser(sharing));
});
Config.Sharing.passwordRequired = true;
const result = await shareLogin(server, sharing.sharingKey);
result.should.have.status(401);
result.body.should.be.a('object');
result.body.error.should.be.a('object');
should.equal(result.body.error.code, ErrorCodes.CREDENTIAL_NOT_FOUND);
});
});

View File

@ -125,9 +125,9 @@ describe('UserRouter', () => {
it('it should authenticate as user with sharing key', async () => {
Config.Users.authenticationRequired = true;
Config.Sharing.enabled = true;
Config.Sharing.passwordProtected = true;
Config.Sharing.passwordRequired = true;
const sharingKey = (await RouteTestingHelper.createSharing(testUser)).sharingKey;
const sharingKey = (await RouteTestingHelper.createSharing(testUser, 'pass')).sharingKey;
const loginRes = await login(server);
@ -146,7 +146,7 @@ describe('UserRouter', () => {
it('it should authenticate with sharing key', async () => {
Config.Users.authenticationRequired = true;
Config.Sharing.enabled = true;
Config.Sharing.passwordProtected = true;
Config.Sharing.passwordRequired = false;
const sharing = (await RouteTestingHelper.createSharing(testUser));
@ -161,7 +161,7 @@ describe('UserRouter', () => {
it('it should not authenticate with sharing key without password', async () => {
Config.Users.authenticationRequired = true;
Config.Sharing.enabled = true;
Config.Sharing.passwordProtected = true;
Config.Sharing.passwordRequired = true;
const sharing = (await RouteTestingHelper.createSharing(testUser, 'pass_secret'));