Skip to content

BREAKING CHANGE: Update WebApi routes for security and RESTful best practices#417

Merged
glahaye merged 11 commits intomicrosoft:mainfrom
glahaye:server_routes
Oct 3, 2023
Merged

BREAKING CHANGE: Update WebApi routes for security and RESTful best practices#417
glahaye merged 11 commits intomicrosoft:mainfrom
glahaye:server_routes

Conversation

@glahaye
Copy link
Copy Markdown
Contributor

@glahaye glahaye commented Sep 29, 2023

Motivation and Context

The REST routes have grown organically and don't represent the best REST practices.
Also, user ID is sometimes taken from routes instead of verified tokens.
This PR is to address these two points.

Description

  • Changed routes
  • Renamed classes accordingly
  • Hid chat archive (formally "bot") schema settings from user
  • Simplified IContentSafetyService interface and related dependency injection
  • Removed ConfigureAwait(false) instances (they have no effect in ASP.Net Core apps)
  • Moved isContentSafetyEnabled to ServiceOptions

Contribution Checklist

@github-actions github-actions Bot added webapp Pull requests that update Typescript code webapi Pull requests that update .net code labels Sep 29, 2023
@glahaye glahaye self-assigned this Sep 29, 2023
@glahaye glahaye added PR: breaking change Pull requests that introduce breaking changes and removed webapp Pull requests that update Typescript code webapi Pull requests that update .net code labels Sep 29, 2023
@glahaye glahaye linked an issue Sep 29, 2023 that may be closed by this pull request
@github-actions github-actions Bot added webapp Pull requests that update Typescript code webapi Pull requests that update .net code labels Sep 30, 2023
Comment thread webapi/Controllers/ChatArchiveController.cs Outdated
Comment thread webapi/Controllers/ChatArchiveController.cs Outdated
Comment thread webapi/Controllers/ServiceInfoController.cs Outdated
Comment thread webapi/Models/Response/CreateChatResponse.cs Outdated
Comment thread webapi/Skills/ChatSkills/SemanticMemoryRetriever.cs
Comment thread webapi/Models/Response/ServiceInfoResponse.cs Outdated
Comment thread webapp/src/libs/services/BotService.ts
Comment thread webapp/src/libs/services/ChatService.ts Outdated
Comment thread webapp/src/libs/services/BotService.ts Outdated
crickman
crickman previously approved these changes Oct 2, 2023
Copy link
Copy Markdown
Contributor

@crickman crickman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked areas for which I am familiar (memory stuff)...looks good.,

Comment thread webapi/Controllers/ChatArchiveController.cs Outdated
Comment thread webapi/Controllers/ChatController.cs Outdated
Comment thread webapi/Options/ChatArchiveSchemaInfo.cs
Comment thread webapi/Controllers/ChatController.cs Outdated
Comment thread webapi/Controllers/ChatController.cs
glahaye and others added 2 commits October 2, 2023 16:38
Co-authored-by: Desmond Howard <dehoward@microsoft.com>
glahaye and others added 5 commits October 2, 2023 16:54
Co-authored-by: Desmond Howard <dehoward@microsoft.com>
Co-authored-by: Desmond Howard <dehoward@microsoft.com>
Co-authored-by: Desmond Howard <dehoward@microsoft.com>
@glahaye glahaye requested a review from dehoward October 3, 2023 01:16
@glahaye glahaye changed the title Update WebApi routes for security and RESTful best practices BREAKING CHANGE: Update WebApi routes for security and RESTful best practices Oct 3, 2023
@glahaye glahaye added this pull request to the merge queue Oct 3, 2023
Merged via the queue into microsoft:main with commit 56b595f Oct 3, 2023
@glahaye glahaye deleted the server_routes branch October 3, 2023 18:46
teamleader-dev pushed a commit to vlink-group/chat-copilot that referenced this pull request Oct 7, 2024
…ractices (microsoft#417)

### Motivation and Context
The REST routes have grown organically and don't represent the best REST
practices.
Also, user ID is sometimes taken from routes instead of verified tokens.
This PR is to address these two points.

### Description
- Changed routes
- Renamed classes accordingly
- Hid chat archive (formally "bot") schema settings from user
- Simplified IContentSafetyService interface and related dependency
injection
- Removed ConfigureAwait(false) instances (they have no effect in
ASP.Net Core apps)
- Moved isContentSafetyEnabled to ServiceOptions

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Desmond Howard <dehoward@microsoft.com>
kb0039 pushed a commit to aaronba/chat-copilot that referenced this pull request Jan 8, 2025
…ractices (microsoft#417)

### Motivation and Context
The REST routes have grown organically and don't represent the best REST
practices.
Also, user ID is sometimes taken from routes instead of verified tokens.
This PR is to address these two points.

### Description
- Changed routes
- Renamed classes accordingly
- Hid chat archive (formally "bot") schema settings from user
- Simplified IContentSafetyService interface and related dependency
injection
- Removed ConfigureAwait(false) instances (they have no effect in
ASP.Net Core apps)
- Moved isContentSafetyEnabled to ServiceOptions

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Desmond Howard <dehoward@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: breaking change Pull requests that introduce breaking changes webapi Pull requests that update .net code webapp Pull requests that update Typescript code

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

Update WebApi routes for security and RESTful best practices.

6 participants