Skip to content

Commit d3aaa4b

Browse files
Copilotsezanzeb
andauthored
fix: rating service permission checks, ownership enforcement, and value validation
Agent-Logs-Url: https://github.com/hackaburg/tilt/sessions/8bee7ebf-ee16-40be-97ae-055fc8d2b76c Co-authored-by: sezanzeb <28510156+sezanzeb@users.noreply.github.com>
1 parent 92562a9 commit d3aaa4b

4 files changed

Lines changed: 2084 additions & 1796 deletions

File tree

backend/src/controllers/dto.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import {
1010
IsOptional,
1111
IsString,
1212
IsUrl,
13+
Max,
1314
MaxLength,
15+
Min,
1416
MinLength,
1517
ValidateNested,
1618
} from "class-validator";
@@ -602,6 +604,8 @@ export class RatingDTO {
602604
@ValidateNested()
603605
public critera!: CriteriaDTO;
604606
@Expose()
605-
// 1 - 5
607+
@IsInt()
608+
@Min(1)
609+
@Max(5)
606610
public rating!: number;
607611
}

backend/src/controllers/rating-controller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class RatingController {
3636
@CurrentUser() user: User,
3737
): Promise<RatingDTO> {
3838
const rating = convertBetweenEntityAndDTO(ratingDTO, Rating);
39-
const createdRating = await this._ratings.createRating(rating);
39+
const createdRating = await this._ratings.createRating(rating, user);
4040
return convertBetweenEntityAndDTO(createdRating, RatingDTO);
4141
}
4242

backend/src/services/rating-service.ts

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export interface IRatingService extends IService {
3535
/**
3636
* Delete single rating by id
3737
*/
38-
deleteRatingByID(id: number, currentUserId: User): Promise<void>;
38+
deleteRatingByID(id: number, currentUser: User): Promise<void>;
3939
//
4040
//
4141
/**
@@ -102,20 +102,18 @@ export class RatingService implements IRatingService {
102102
* @param rating The rating to update
103103
*/
104104
public async updateRating(rating: Rating, user: User): Promise<Rating> {
105-
// TODO validate, throw Errors
106-
107105
const originRating = await this._ratings.findOneBy({ id: rating.id });
108106

109-
// TODO only if user matches
110-
await this.checkPermission(rating, user);
111107
if (!originRating) {
112108
throw new NotFoundError("Rating not found");
113109
}
114-
const originRatingUser = originRating.user;
115-
if (user.id != originRatingUser.id) {
116-
throw new Error("")
110+
111+
if (user.id !== originRating.user.id) {
112+
throw new ForbiddenError("You can only update your own ratings");
117113
}
118114

115+
await this.checkPermission(rating, user);
116+
119117
return this._ratings.save(rating);
120118
}
121119

@@ -124,8 +122,7 @@ export class RatingService implements IRatingService {
124122
* @param rating The rating to create
125123
*/
126124
public async createRating(rating: Rating, user: User): Promise<Rating> {
127-
// TODO validate
128-
this.checkPermission(rating, user);
125+
await this.checkPermission(rating, user);
129126
return this._ratings.save(rating);
130127
}
131128

@@ -142,44 +139,43 @@ export class RatingService implements IRatingService {
142139
* Deletes a rating by its id.
143140
* @param id The id of the rating
144141
*/
145-
public async deleteRatingByID(id: number, currentUserId: User): Promise<void> {
142+
public async deleteRatingByID(id: number, currentUser: User): Promise<void> {
146143
const rating = await this._ratings.findOneBy({ id });
147144

148145
if (!rating) {
149146
throw new NotFoundError("Rating not found");
150147
}
151148

152-
await this.checkPermission(rating, currentUserId);
149+
if (currentUser.id !== rating.user.id) {
150+
throw new ForbiddenError("You can only delete your own ratings");
151+
}
152+
153+
await this.checkPermission(rating, currentUser);
153154

154155
await this._ratings.delete(id);
155156

156157
return Promise.resolve();
157158
}
158159

159160
private async checkPermission(rating: Rating, user: User): Promise<void> {
160-
// TODO use this._database.blabla instead of _settings and such
161-
162161
const settings = await this._settings.getSettings();
163162
if (!settings.allowRating) {
164-
// TODO test
165-
throw new ForbiddenError("Cannot create rating due to application settings")
163+
throw new ForbiddenError("Rating is not allowed due to application settings")
166164
}
167165

168166
const project = await this._projects.findOneBy({ id: rating.project.id });
169167
if (!project) {
170168
throw new NotFoundError("Project not found");
171169
}
172170
if (!project.allowRating) {
173-
// TODO test
174-
throw new ForbiddenError("Creating a rating for this project is not allowed")
171+
throw new ForbiddenError("Rating this project is not allowed")
175172
}
176173

177174
const team = await this._teams.findOneBy({ id: project.team.id })
178175
if (!team) {
179176
throw new NotFoundError("Team not found");
180177
}
181178
if (team.users.includes(user.id)) {
182-
// TODO test
183179
throw new ForbiddenError("You can't rate your own project")
184180
}
185181
}

0 commit comments

Comments
 (0)