Development: Test VSCode plugin support
#8840
Conversation
github-actions
bot
added
server
|
As this comment was not addressed in #8819 (and this is the PR which contains those changes now), I'll leave it here again so it is not lost/forgotten about:
|
theres gonna be a discussion on monday concerning this issue |
but to answer that its just the fetch API which is able to handle cookies. the problem is though that vscode plugins dont support cookie nor local storage |
I've looked into the fetch docs and it provides the |
It indeed does provide this method. But still if you call the method or in general print all headers the |
Screen.Recording.2024-06-22.at.16.32.17.movthats a request on the {base_url}/api/public/authenticate endpoint |
|
According to several online sources it seems like you have to set the |
I'm aware that the fetch API does support cookies. But the vscode extension environment does not. So cookies wont work in a VSCode extension. |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
github-actions
bot
added
the
client
WalkthroughThe pull request introduces multiple changes across various classes in the project. Key modifications include enhancements to CORS logging in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
janthoXO
temporarily deployed
to
artemis-test9.artemis.cit.tum.de
GitHub Actions
Inactive
github-actions
bot
added
lock:artemis-test9
and removed
deploy:artemis-test9
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
🧹 Outside diff range comments (3)
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java (1)
Line range hint
242-247: Potential security concern in toString() methodThe
toString()method includes sensitive information likebuildScript. Consider excluding sensitive data from thetoString()output to prevent accidental exposure of this information in logs or error messages.You could modify the
toString()method to exclude or mask sensitive information. For example:@Override public String toString() { return "BuildJobConfig{" + "id=" + getId() + ", sequentialTestRuns=" + sequentialTestRuns + ", branch='" + branch + '\'' + ", checkoutSolutionRepository=" + checkoutSolutionRepository + ", checkoutPath='" + checkoutPath + '\'' + ", timeoutSeconds=" + timeoutSeconds + ", dockerFlags='" + dockerFlags + '\'' + ", testwiseCoverageEnabled=" + testwiseCoverageEnabled + ", theiaImage='" + theiaImage + '\'' + ", allowBranching=" + allowBranching + ", branchRegex='" + branchRegex + '\'' + '}'; }src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
Line range hint
1-279: Overall assessment: Changes align with PR objectives and enhance security.The introduction of
CsrfArtemisFilterand its integration into the security filter chain aligns well with the PR objectives of enhancing CSRF security. The implementation follows good coding practices and maintains the single responsibility principle for the affected method.To fully align with our coding guidelines, consider implementing constructor injection for the new
CsrfArtemisFilteras suggested earlier. This change would improve testability and adhere more closely to our dependency injection practices.The changes effectively contribute to the overall security enhancements mentioned in the PR objectives, particularly in strengthening CSRF protection.
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
Line range hint
301-369: Improved feedback request conditions in assureConditionsSatisfied method.The
assureConditionsSatisfiedmethod has been updated with new conditions for requesting automatic non-graded feedback. This includes checks for:
- Programming exercise specific conditions
- Due date check
- Previous feedback request check
- Rate limiting (max 10 requests)
- Check for existing Athena result on the latest submission
These additions improve the robustness of the feedback request process.
However, consider extracting some of these checks into separate methods to improve readability and maintainability.
For example:
private isProgrammingExerciseConditionsSatisfied(): boolean { if (this.exercise.type === ExerciseType.PROGRAMMING) { const latestResult = this.gradedParticipation?.results && this.gradedParticipation.results.find(({ assessmentType }) => assessmentType === AssessmentType.AUTOMATIC); const someHiddenTestsPassed = latestResult?.score !== undefined; const testsNotPassedWarning = this.translateService.instant('artemisApp.exercise.notEnoughPoints'); if (!someHiddenTestsPassed) { window.alert(testsNotPassedWarning); return false; } } return true; } private isWithinDueDate(): boolean { const afterDueDate = !this.exercise.dueDate || dayjs().isSameOrAfter(this.exercise.dueDate); if (afterDueDate) { const dueDateWarning = this.translateService.instant('artemisApp.exercise.feedbackRequestAfterDueDate'); this.alertService.warning(dueDateWarning); return false; } return true; } // ... other extracted methods assureConditionsSatisfied(): boolean { this.updateParticipations(); return this.isProgrammingExerciseConditionsSatisfied() && this.isWithinDueDate() && this.isNotAlreadyRequested() && this.isWithinRateLimit() && !this.hasAthenaResultForlatestSubmission(); }This refactoring would make the
assureConditionsSatisfiedmethod more readable and easier to maintain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (2)
src/main/resources/config/application-dev.ymlis excluded by!**/*.ymlsrc/main/resources/config/application.ymlis excluded by!**/*.yml
📒 Files selected for processing (23)
- src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (3 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/config/WebConfigurer.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/security/filter/CsrfArtemisFilter.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/security/jwt/JWTCookieService.java (3 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/security/jwt/JWTFilter.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/security/jwt/TokenProvider.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/web/open/PublicUserJwtResource.java (5 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseBuildConfigRepository.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (7 hunks)
- src/main/webapp/app/app.module.ts (2 hunks)
- src/main/webapp/app/core/auth/account.service.ts (1 hunks)
- src/main/webapp/app/core/csrf/csrf.interceptor.ts (1 hunks)
- src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (2 hunks)
- src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (0 hunks)
- src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1 hunks)
- src/main/webapp/app/shared/components/code-button/code-button.component.html (1 hunks)
- src/main/webapp/app/shared/components/code-button/code-button.component.ts (6 hunks)
- src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (0 hunks)
- src/test/javascript/spec/component/shared/code-button.component.spec.ts (5 hunks)
- src/test/javascript/spec/helpers/mocks/service/mock-account.service.ts (1 hunks)
- src/test/javascript/spec/helpers/mocks/service/mock-programming-exercise.service.ts (2 hunks)
💤 Files with no reviewable changes (2)
- src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html
- src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (21)
src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/config/WebConfigurer.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/security/filter/CsrfArtemisFilter.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/security/jwt/JWTCookieService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/security/jwt/JWTFilter.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/security/jwt/TokenProvider.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/core/web/open/PublicUserJwtResource.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseBuildConfigRepository.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/app.module.ts (1)
src/main/webapp/app/core/auth/account.service.ts (1)
src/main/webapp/app/core/csrf/csrf.interceptor.ts (1)
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
src/main/webapp/app/shared/components/code-button/code-button.component.html (1)
Pattern
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/shared/components/code-button/code-button.component.ts (1)
src/test/javascript/spec/component/shared/code-button.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/helpers/mocks/service/mock-account.service.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/helpers/mocks/service/mock-programming-exercise.service.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
📓 Learnings (3)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (3)
Learnt from: Hialus PR: ls1intum/Artemis#8607 File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64 Timestamp: 2024-06-15T20:04:18.637Z Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.Learnt from: Hialus PR: ls1intum/Artemis#8607 File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64 Timestamp: 2024-10-08T15:35:42.972Z Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.Learnt from: Hialus PR: ls1intum/Artemis#8607 File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64 Timestamp: 2024-10-08T15:35:48.767Z Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.src/main/webapp/app/core/auth/account.service.ts (4)
Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/core/auth/account.service.ts:383-390 Timestamp: 2024-10-08T11:30:31.107Z Learning: In `src/main/webapp/app/core/auth/account.service.ts`, error handling for service methods like `rekeyCookieToBearerToken` should be handled in the using component, not within the service method.Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/core/auth/account.service.ts:383-390 Timestamp: 2024-10-08T15:35:52.595Z Learning: In `src/main/webapp/app/core/auth/account.service.ts`, error handling for service methods like `rekeyCookieToBearerToken` should be handled in the using component, not within the service method.Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/core/auth/account.service.ts:383-390 Timestamp: 2024-10-08T15:35:48.768Z Learning: In 'src/main/webapp/app/core/auth/account.service.ts', the `rekeyCookieToBearerToken` method receives a plain string response from the server (not JSON), hence `responseType` should be set to `'text'`.Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/core/auth/account.service.ts:383-390 Timestamp: 2024-10-04T12:59:06.788Z Learning: In 'src/main/webapp/app/core/auth/account.service.ts', the `rekeyCookieToBearerToken` method receives a plain string response from the server (not JSON), hence `responseType` should be set to `'text'`.src/main/webapp/app/shared/components/code-button/code-button.component.html (3)
Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/shared/components/code-button/code-button.component.html:102-0 Timestamp: 2024-10-08T15:35:48.768Z Learning: For the `startOnlineIDE()` function that causes an instantaneous redirect, adding a loading state to its button is unnecessary.Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/shared/components/code-button/code-button.component.html:102-0 Timestamp: 2024-10-08T15:35:42.972Z Learning: For the `startOnlineIDE()` function that causes an instantaneous redirect, adding a loading state to its button is unnecessary.Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/shared/components/code-button/code-button.component.html:102-0 Timestamp: 2024-09-28T13:48:54.189Z Learning: For the `startOnlineIDE()` function that causes an instantaneous redirect, adding a loading state to its button is unnecessary.
🪛 Biome
src/main/webapp/app/shared/components/code-button/code-button.component.ts
[error] 363-363: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
src/test/javascript/spec/component/shared/code-button.component.spec.ts
[error] 602-602: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
🔇 Additional comments (25)
src/main/webapp/app/core/csrf/csrf.interceptor.ts (2)
1-2: LGTM: Imports are correct and follow guidelines.The import statements are appropriate for the CSRF interceptor implementation and use single quotes as per the coding guidelines.
4-4: LGTM: Class declaration follows best practices and guidelines.The
CsrfInterceptorclass is correctly exported and implements theHttpInterceptorinterface. The class name uses PascalCase, adhering to the coding guidelines.src/test/javascript/spec/helpers/mocks/service/mock-programming-exercise.service.ts (1)
5-5: LGTM: New import added correctly.The new import for
ProgrammingExerciseBuildConfigis correctly added and follows the existing import style in the file.src/main/java/de/tum/cit/aet/artemis/core/security/jwt/JWTCookieService.java (2)
20-22: Improved constructor designThe simplified constructor aligns better with the single responsibility principle by removing the unnecessary
Environmentdependency. This change enhances the class's focus on JWT cookie-related operations.
Line range hint
1-72: Overall assessment: Well-structured and secure implementationThe changes to
JWTCookieServicehave improved its design and functionality:
- The constructor is now more focused, adhering to the single responsibility principle.
- The new
buildTheiaCookiemethod extends functionality while maintaining good coding practices.- Cookie security settings have been enhanced to support cross-site requests securely.
These modifications align well with the provided coding guidelines, particularly in terms of REST API best practices, security considerations, and code organization. The suggestions provided are minor and aimed at further improving flexibility and documentation.
src/main/webapp/app/app.module.ts (1)
29-29: LGTM: Import statement correctly addedThe import of
HTTP_INTERCEPTORSfrom@angular/common/httpis correctly placed and follows Angular style guidelines. This import is necessary for adding the CSRF interceptor to the providers.src/main/java/de/tum/cit/aet/artemis/core/config/WebConfigurer.java (2)
123-124: LGTM: Improved CORS configuration check.The expanded conditional check now considers both
getAllowedOriginsandgetAllowedOriginPatterns. This change provides more flexibility in CORS configuration and aligns with the PR objectives to adjust CORS settings.
120-124: Overall assessment: CORS configuration improvements look good.The changes to the
corsFiltermethod enhance the CORS configuration flexibility and provide better visibility into the settings. These modifications align well with the PR objectives to adjust CORS configuration and improve security measures.A minor suggestion was made to adjust the log level for the new logging statements. Otherwise, the changes appear to be correct and improve the functionality as intended.
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java (1)
28-28: LGTM: Improved JSON serialization handlingThe addition of
@JsonIgnoreProperties(value = { "programmingExercise" })is a good change. It prevents theprogrammingExercisefield from being included in JSON representations of this class, which can help avoid circular references during serialization. This aligns with the REST API best practice of using minimal DTOs as mentioned in the coding guidelines.src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
38-38: LGTM: New import for CsrfArtemisFilter.The import statement for
CsrfArtemisFilteris correctly placed and follows the naming conventions.src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (3)
14-14: LGTM: Import statement updated correctly.The removal of the
faDesktopicon import is consistent with the changes made to the component, likely related to the removal of Theia-related functionality.
Line range hint
93-126: LGTM: Theia-related logic removed from ngOnInit.The removal of Theia-related logic from the
ngOnInitmethod is consistent with the overall changes to the component. The remaining initialization logic for other exercise types looks correct.
21-21: Verify usage of remaining IDE-related properties.The removal of Theia-related properties (
theiaEnabledandtheiaPortalURL) is consistent with the changes mentioned in the summary. However,localVCEnabledandathenaEnabledproperties are still present.Please ensure that these remaining properties are still used and necessary. If not, consider removing them as well.
✅ Verification successful
IDE-related properties are still in use.
The properties
localVCEnabledandathenaEnabledare found in multiple components within the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify usage of localVCEnabled and athenaEnabled properties rg --type typescript "localVCEnabled|athenaEnabled" src/main/webappLength of output: 104
Script:
#!/bin/bash # Verify usage of localVCEnabled and athenaEnabled properties in TypeScript files rg "localVCEnabled|athenaEnabled" src/main/webapp --glob "*.ts"Length of output: 5562
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (2)
30-30: LGTM: New import statement is correctly implemented.The import for
ProgrammingExerciseBuildConfigis properly placed and follows the correct naming convention.
30-30: Summary: Excellent addition of build configuration retrieval functionality.The new import and
getBuildConfigmethod have been implemented cleanly and efficiently. They follow Angular best practices and the existing coding style of the file. These changes enhance the service's capabilities without disrupting the existing codebase.Great job on maintaining code quality while expanding functionality!
Also applies to: 664-667
src/main/java/de/tum/cit/aet/artemis/core/security/filter/CsrfArtemisFilter.java (1)
38-38: Verify All API Endpoints Are ProtectedThe
shouldNotFiltermethod excludes requests that do not start with/api/, meaning only/api/**endpoints are protected by this filter. It's important to verify that there are no other endpoints that require CSRF protection but do not start with/api/.Run the following script to identify endpoints outside
/api/:This script searches for REST controller methods with mappings not starting with
/api/. Verify if any of these endpoints should be protected by the CSRF filter.src/main/java/de/tum/cit/aet/artemis/core/security/jwt/JWTFilter.java (1)
36-41: Handle Missing or Invalid JWT GracefullyEnsure that the application handles missing or invalid JWT tokens appropriately. If the token is invalid or absent, and the request requires authentication, the user should receive a proper error response.
To verify the behavior, you can test unauthorized requests:
src/main/java/de/tum/cit/aet/artemis/core/security/jwt/TokenProvider.java (1)
98-99: Ensure correct method delegationThe
createToken(Authentication authentication, boolean rememberMe)method now delegates token creation to the new method with a duration parameter. This change maintains the original functionality while promoting code reuse.src/main/java/de/tum/cit/aet/artemis/core/web/open/PublicUserJwtResource.java (6)
5-7: Approval of Added ImportsThe imports of
Duration,Date, andMapare appropriate and necessary for the new functionality implemented in the class.
33-33: Approval of Added Import forRequestParamThe import of
RequestParamis correctly added to support request parameter handling in the new methodgetTheiaToken.
40-40: Approval of Security-Related ImportsThe added imports for
EnforceAtLeastStudent,JWTFilter, andTokenProviderare necessary for implementing the enhanced security features.Also applies to: 43-44
59-60: Proper Use of Dependency Injection forTokenProviderInjecting
TokenProvidervia constructor injection aligns with best practices and the project's dependency injection guidelines.Also applies to: 65-67
59-60: Field Access Modifiers Align with Best PracticesThe
tokenProviderfield is declared asprivate final, adhering to the principle of least privilege and ensuring immutability.
119-122: Verification of JWT Extraction LogicPlease verify that the
JWTFilter.extractValidJwtmethod correctly handles edge cases and does not introduce security vulnerabilities.Verification Script:
This script searches for the implementation and all usages of
extractValidJwtto help ensure it is implemented securely and used consistently.src/main/webapp/app/shared/components/code-button/code-button.component.ts (1)
360-375:⚠️ Potential issueAvoid using non-null assertion operator; handle potential null values
Using the non-null assertion operator
!inthis.exercise.id!(line 363) can lead to runtime errors ifthis.exercise.idisnullorundefined. It's safer to add a null check to ensurethis.exercise.idis defined before using it.Apply this diff to fix the issue:
private initTheia(profileInfo: ProfileInfo) { if (profileInfo.activeProfiles?.includes(PROFILE_THEIA) && this.exercise) { + if (this.exercise.id) { this.programmingExerciseService.getBuildConfig(this.exercise.id).subscribe((buildConfig) => { if (this.exercise) { this.exercise.buildConfig = buildConfig; // Set variables now, sanitize later on this.theiaPortalURL = profileInfo.theiaPortalURL ?? ''; // Verify that all conditions are met if (this.theiaPortalURL !== '' && this.exercise.allowOnlineIde && this.exercise.buildConfig?.theiaImage) { this.theiaEnabled = true; } } }); + } } }⛔ Skipped due to learnings
Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170 Timestamp: 2024-10-08T15:35:42.972Z Learning: In `code-button.component.ts`, `this.exercise.id` is checked before use, so it's acceptable to use the non-null assertion operator `!` on `this.exercise.id!`.Learnt from: iyannsch PR: ls1intum/Artemis#9379 File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170 Timestamp: 2024-09-30T08:34:49.363Z Learning: In `code-button.component.ts`, `this.exercise.id` is checked before use, so it's acceptable to use the non-null assertion operator `!` on `this.exercise.id!`.Learnt from: florian-glombik PR: ls1intum/Artemis#8858 File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:97-107 Timestamp: 2024-10-08T15:35:48.768Z Learning: In the context of PR ls1intum/Artemis#8858, avoid suggesting the removal of non-null assertions.Learnt from: florian-glombik PR: ls1intum/Artemis#8858 File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:97-107 Timestamp: 2024-10-08T15:35:42.972Z Learning: In the context of PR ls1intum/Artemis#8858, avoid suggesting the removal of non-null assertions.Learnt from: florian-glombik PR: ls1intum/Artemis#8858 File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:97-107 Timestamp: 2024-07-09T19:10:33.905Z Learning: In the context of PR ls1intum/Artemis#8858, avoid suggesting the removal of non-null assertions.Learnt from: florian-glombik PR: ls1intum/Artemis#8858 File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:97-111 Timestamp: 2024-10-08T15:35:52.595Z Learning: In the context of PR ls1intum/Artemis#8858, non-null assertions should not be flagged or suggested for removal.Learnt from: florian-glombik PR: ls1intum/Artemis#8858 File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:97-111 Timestamp: 2024-07-09T19:06:18.665Z Learning: In the context of PR ls1intum/Artemis#8858, non-null assertions should not be flagged or suggested for removal.🧰 Tools
🪛 Biome
[error] 363-363: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
| intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> { | ||
| // https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#employing-custom-request-headers-for-ajaxapi | ||
| // Clone the request and add the CSRF token header | ||
| const csrfReq = req.clone({ headers: req.headers.set('X-ARTEMIS-CSRF', 'Dennis ist schuld') }); | ||
|
|
||
| return next.handle(csrfReq); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Security Issue: Hardcoded CSRF Token Detected
The intercept method in csrf.interceptor.ts uses a hardcoded CSRF token 'Dennis ist schuld'. This practice compromises the security mechanism intended to prevent CSRF attacks.
Recommendation:
-
Replace the hardcoded token with a dynamically generated CSRF token.
import { Injectable } from '@angular/core'; import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http'; import { Observable } from 'rxjs'; @Injectable() export class CsrfInterceptor implements HttpInterceptor { constructor(private csrfService: CsrfService) {} intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> { const csrfToken = this.csrfService.getToken(); const csrfReq = req.clone({ headers: req.headers.set('X-ARTEMIS-CSRF', csrfToken) }); return next.handle(csrfReq); } }
This change ensures that each session utilizes a unique and securely generated CSRF token, significantly strengthening the application's defense against CSRF attacks.
🔗 Analysis chain
Security concern: Replace hardcoded CSRF token with a dynamically generated value.
The intercept method correctly adds a custom CSRF header to all requests. However, using a hardcoded value ('Dennis ist schuld') for the CSRF token significantly reduces its effectiveness in preventing CSRF attacks.
To improve security, consider the following:
- Generate a unique CSRF token for each session.
- Store the token securely (e.g., in a service or secure storage).
- Retrieve the token dynamically in the interceptor.
Here's a suggested improvement:
import { Injectable } from '@angular/core';
import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http';
import { Observable } from 'rxjs';
@Injectable()
export class CsrfInterceptor implements HttpInterceptor {
constructor(private csrfService: CsrfService) {}
intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
const csrfToken = this.csrfService.getToken();
const csrfReq = req.clone({ headers: req.headers.set('X-ARTEMIS-CSRF', csrfToken) });
return next.handle(csrfReq);
}
}This approach ensures that a unique and securely generated CSRF token is used for each request, significantly enhancing the application's security against CSRF attacks.
To ensure that the CSRF token is not hardcoded in other parts of the codebase, run the following script:
This script will help identify any other instances where the CSRF token might be hardcoded, allowing for a comprehensive security review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for hardcoded CSRF tokens in TypeScript files
rg --type typescript "'X-ARTEMIS-CSRF'" -C 3
Length of output: 87
Script:
#!/bin/bash
# Search for hardcoded CSRF tokens in TypeScript files using file extensions
rg --glob '*.ts' --glob '*.tsx' 'X-ARTEMIS-CSRF' -C 3
Length of output: 854
| /** | ||
| * Find a build config by its programming exercise's id and throw an Exception if it cannot be found | ||
| * | ||
| * @param programmingExerciseId of the programming exercise. | ||
| * @return The programming exercise related to the given id | ||
| */ | ||
| default ProgrammingExerciseBuildConfig findByExerciseIdElseThrow(long programmingExerciseId) { | ||
| return getValueElseThrow(findByProgrammingExerciseId(programmingExerciseId)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider enhancing the Javadoc.
The new method findByExerciseIdElseThrow is well-implemented and aligns with the coding guidelines. It promotes code reuse, has a clear single responsibility, and uses appropriate error handling.
Consider enhancing the Javadoc by specifying the exception type that may be thrown:
/**
* Find a build config by its programming exercise's id and throw an Exception if it cannot be found
*
* @param programmingExerciseId of the programming exercise.
* @return The programming exercise related to the given id
+ * @throws javax.persistence.EntityNotFoundException if the build config cannot be found
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Find a build config by its programming exercise's id and throw an Exception if it cannot be found | |
| * | |
| * @param programmingExerciseId of the programming exercise. | |
| * @return The programming exercise related to the given id | |
| */ | |
| default ProgrammingExerciseBuildConfig findByExerciseIdElseThrow(long programmingExerciseId) { | |
| return getValueElseThrow(findByProgrammingExerciseId(programmingExerciseId)); | |
| } | |
| /** | |
| * Find a build config by its programming exercise's id and throw an Exception if it cannot be found | |
| * | |
| * @param programmingExerciseId of the programming exercise. | |
| * @return The programming exercise related to the given id | |
| * @throws javax.persistence.EntityNotFoundException if the build config cannot be found | |
| */ | |
| default ProgrammingExerciseBuildConfig findByExerciseIdElseThrow(long programmingExerciseId) { | |
| return getValueElseThrow(findByProgrammingExerciseId(programmingExerciseId)); | |
| } |
| @@ -42,4 +42,5 @@ export class MockAccountService implements IAccountService { | |||
| addSshPublicKey = (sshPublicKey: string) => of(); | |||
| getVcsAccessToken = (participationId: number) => of(); | |||
| createVcsAccessToken = (participationId: number) => of(); | |||
| rekeyCookieToBearerToken = () => of(); | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider adding a TODO comment for future implementation.
The rekeyCookieToBearerToken method is correctly implemented as a stub for the mock service. It follows the existing coding style and is consistent with other methods in the class.
To improve clarity for future developers, consider adding a TODO comment:
rekeyCookieToBearerToken = () => of(); // TODO: Implement mock behavior if needed for specific tests| @@ -18,6 +19,7 @@ export class MockProgrammingExerciseService { | |||
| deleteTasksWithSolutionEntries = (exerciseId: number) => of(); | |||
| getDiffReport = (exerciseId: number) => of({}); | |||
| getBuildLogStatistics = (exerciseId: number) => of({}); | |||
| getBuildConfig = (exerciseId: number) => of({}); | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider improving type safety and mock implementation.
While the new getBuildConfig method follows the pattern of other methods in this mock service, there are a few suggestions for improvement:
- Add a return type annotation to enhance type safety. It should likely be
Observable<ProgrammingExerciseBuildConfig>. - Consider returning a mock
ProgrammingExerciseBuildConfigobject instead of an empty object to better simulate the real service behavior.
Here's a suggested improvement:
getBuildConfig = (exerciseId: number): Observable<ProgrammingExerciseBuildConfig> => of({
// Add mock properties here
// For example:
// id: exerciseId,
// someProperty: 'mockValue',
});This change would make the mock more useful for testing scenarios that depend on the structure of ProgrammingExerciseBuildConfig.
| /** | ||
| * Builds the cookie with Theia flag | ||
| * | ||
| * @param duration the duration of the cookie and the jwt | ||
| * @return the login ResponseCookie containing the JWT | ||
| */ | ||
| public ResponseCookie buildTheiaCookie(long duration) { | ||
| String jwt = tokenProvider.createToken(SecurityContextHolder.getContext().getAuthentication(), duration, "THEIA"); | ||
| return buildJWTCookie(jwt, Duration.of(duration, ChronoUnit.MILLIS)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
New method for Theia-specific cookie creation
The buildTheiaCookie method is a well-designed addition that follows good coding practices:
- It adheres to the single responsibility principle.
- It reuses existing code (
buildJWTCookie), following the DRY principle. - The method signature uses a primitive type for the duration parameter.
Consider enhancing the method's documentation by specifying the unit of the duration parameter (e.g., milliseconds, seconds) for clarity.
| form.method = 'GET'; | ||
| form.action = this.theiaPortalURL; | ||
| form.target = newWindow.name; |
There was a problem hiding this comment.
Use 'POST' method for form submission to protect sensitive data
Currently, the form uses the 'GET' method, which appends form data to the URL. Since sensitive data like gitUri and artemisToken are being submitted, it's recommended to use 'POST' to prevent these values from appearing in the URL and browser history.
Apply this diff to fix the issue:
-form.method = 'GET';
+form.method = 'POST';
form.action = this.theiaPortalURL;
form.target = newWindow.name;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| form.method = 'GET'; | |
| form.action = this.theiaPortalURL; | |
| form.target = newWindow.name; | |
| form.method = 'POST'; | |
| form.action = this.theiaPortalURL; | |
| form.target = newWindow.name; |
| } | ||
|
|
||
| async startOnlineIDE() { | ||
| const artemisToken: string = (await this.accountService.rekeyCookieToBearerToken().toPromise()) ?? ''; |
There was a problem hiding this comment.
Handle potential errors during token retrieval
The asynchronous call to retrieve artemisToken may fail, leading to unhandled exceptions. Consider adding error handling to manage potential failures when obtaining the token.
Apply this diff to improve error handling:
-const artemisToken: string = (await this.accountService.rekeyCookieToBearerToken().toPromise()) ?? '';
+let artemisToken: string = '';
+try {
+ artemisToken = (await this.accountService.rekeyCookieToBearerToken().toPromise()) ?? '';
+} catch (error) {
+ console.error('Failed to retrieve Artemis token:', error);
+ // Optionally, notify the user about the failure
+ return;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const artemisToken: string = (await this.accountService.rekeyCookieToBearerToken().toPromise()) ?? ''; | |
| let artemisToken: string = ''; | |
| try { | |
| artemisToken = (await this.accountService.rekeyCookieToBearerToken().toPromise()) ?? ''; | |
| } catch (error) { | |
| console.error('Failed to retrieve Artemis token:', error); | |
| // Optionally, notify the user about the failure | |
| return; | |
| } |
| if (Object.hasOwn(data, key)) { | ||
| const input = Array.from(inputs).find((input) => input.name === key); | ||
| expect(input).toBeDefined(); | ||
| expect(input!.value).toBe(data[key]); |
There was a problem hiding this comment.
Avoid using the non-null assertion operator '!'; replace with a safer alternative
The use of the non-null assertion operator ! on input can lead to runtime errors if input is null or undefined. Since you have already asserted that input is defined with expect(input).toBeDefined();, you can safely access input.value without the non-null assertion.
Apply this diff to fix the issue:
- expect(input!.value).toBe(data[key]);
+ expect(input.value).toBe(data[key]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(input!.value).toBe(data[key]); | |
| expect(input.value).toBe(data[key]); |
🧰 Tools
🪛 Biome
[error] 602-602: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
| /** | ||
| * GET /programming-exercises/:exerciseId/build-config : get the build config of "exerciseId" programmingExercise. | ||
| * | ||
| * @param exerciseId the id of the programmingExercise to retrieve the config for | ||
| * @return the ResponseEntity with status 200 (OK) and with body the programmingExerciseBuildConfig, or with status 404 (Not Found) | ||
| */ | ||
| @GetMapping("programming-exercises/{exerciseId}/build-config") | ||
| @EnforceAtLeastStudentInExercise | ||
| public ResponseEntity<ProgrammingExerciseBuildConfig> getBuildConfig(@PathVariable long exerciseId) { | ||
| log.debug("REST request to get build config of ProgrammingExercise : {}", exerciseId); | ||
| var buildConfig = programmingExerciseBuildConfigRepository.findByExerciseIdElseThrow(exerciseId); | ||
|
|
||
| return ResponseEntity.ok().body(buildConfig); | ||
| } |
There was a problem hiding this comment.
Avoid returning entities directly from REST endpoints
Returning the ProgrammingExerciseBuildConfig entity directly might expose internal implementation details and sensitive information. According to the coding guidelines dtos:{java_records,no_entities,min_data,single_resp}, it's recommended to use a Data Transfer Object (DTO) that contains only the necessary data for the client.
Consider creating a DTO for ProgrammingExerciseBuildConfig and modifying the method to return this DTO. This will help in maintaining a clear separation between your internal data models and the API contracts.
| /** | ||
| * GET /programming-exercises : Queries a programming exercise by its project key. | ||
| * | ||
| * @return the ProgrammingExercise of this project key in an ResponseEntity or 404 Not Found if no exercise exists | ||
| */ | ||
| @GetMapping("programming-exercises/project-key/{projectKey}") | ||
| @EnforceAtLeastStudent | ||
| public ResponseEntity<ProgrammingExercise> getExerciseByProjectKey(@PathVariable String projectKey) { | ||
| final ProgrammingExercise exercise = programmingExerciseRepository.findOneByProjectKeyOrThrow(projectKey, false); | ||
| authCheckService.checkIsAtLeastRoleInExerciseElseThrow(Role.STUDENT, exercise.getId()); | ||
|
|
||
| return ResponseEntity.ok(exercise); | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid returning entities directly from REST endpoints
Returning the ProgrammingExercise entity directly could lead to the exposure of sensitive data and tight coupling between your API and internal data models. As per the coding guidelines dtos:{java_records,no_entities,min_data,single_resp}, it's advisable to use a DTO that includes only the necessary fields for the client.
Consider creating a ProgrammingExerciseDTO and updating the method to return this DTO:
public ResponseEntity<ProgrammingExerciseDTO> getExerciseByProjectKey(@PathVariable String projectKey) {
final ProgrammingExercise exercise = programmingExerciseRepository.findOneByProjectKeyOrThrow(projectKey, false);
authCheckService.checkIsAtLeastRoleInExerciseElseThrow(Role.STUDENT, exercise.getId());
- return ResponseEntity.ok(exercise);
+ ProgrammingExerciseDTO exerciseDTO = new ProgrammingExerciseDTO(exercise);
+ return ResponseEntity.ok(exerciseDTO);
}Committable suggestion was skipped due to low confidence.
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
579-587: Update JavaDoc to reflect the correct endpointThe JavaDoc comment for
getExerciseByProjectKeydoes not accurately represent the actual endpoint pathprogramming-exercises/project-key/{projectKey}. Please update the JavaDoc to match the correct endpoint for clarity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/main/resources/config/application.ymlis excluded by!**/*.yml
📒 Files selected for processing (3)
- src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
📓 Learnings (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (3)
Learnt from: Hialus PR: ls1intum/Artemis#8607 File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64 Timestamp: 2024-06-15T20:04:18.637Z Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.Learnt from: Hialus PR: ls1intum/Artemis#8607 File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64 Timestamp: 2024-10-08T15:35:48.767Z Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.Learnt from: Hialus PR: ls1intum/Artemis#8607 File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64 Timestamp: 2024-10-08T15:35:42.972Z Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
🔇 Additional comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2)
Line range hint
121-179: Integration ofProgrammingExerciseBuildConfigRepositoryis correctThe addition of
ProgrammingExerciseBuildConfigRepositoryand its incorporation into the class and constructor are properly implemented.
505-518: Avoid returning entities directly from REST endpoints
janthoXO
temporarily deployed
to
artemis-test9.artemis.cit.tum.de
GitHub Actions
Inactive
github-actions
bot
added
lock:artemis-test9
and removed
deploy:artemis-test9
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
This PR is deprecated |
THIS PR IS ONLY FOR TESTING PURPOSES ON TEST SERVERS
Description
The PR includes the following main points:
The API looks first if a valid authentication token is provided in the cookie. if not the authorization header is checked.
Mainly the custom header
X-ARTEMIS-CSRFis needed now for every requestthe application.yml defines a "allowed-origin-pattern" which was not checked in the WebConfigurer.java
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests