Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ refactor: refactor azure ad to ms entra id #4168

Merged
merged 10 commits into from
Oct 20, 2024
Merged

Conversation

EINDEX
Copy link
Contributor

@EINDEX EINDEX commented Sep 27, 2024

💻 变更类型 | Change Type

  • ✨ feat
  • 🐛 fix
  • ♻️ refactor
  • 💄 style
  • 👷 build
  • ⚡️ perf
  • 📝 docs
  • 🔨 chore

🔀 变更说明 | Description of Change

Update document for Entra ID.

📝 补充信息 | Additional Information

Copy link

vercel bot commented Sep 27, 2024

@EINDEX is attempting to deploy a commit to the LobeChat Community Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Sep 27, 2024
@lobehubbot
Copy link
Member

👍 @EINDEX

Thank you for raising your pull request and contributing to our Community
Please make sure you have followed our contributing guidelines. We will review it as soon as possible.
If you encounter any problems, please feel free to connect with us.
非常感谢您提出拉取请求并为我们的社区做出贡献,请确保您已经遵循了我们的贡献指南,我们会尽快审查它。
如果您遇到任何问题,请随时与我们联系。

@dosubot dosubot bot added the 📝 Documentation Improvements or additions to documentation | 文档问题 label Sep 27, 2024
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.24%. Comparing base (f68154e) to head (f91c61d).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #4168    +/-   ##
========================================
  Coverage   92.24%   92.24%            
========================================
  Files         493      493            
  Lines       35533    35533            
  Branches     2155     2305   +150     
========================================
  Hits        32777    32777            
  Misses       2756     2756            
Flag Coverage Δ
app 92.24% <ø> (ø)
server 97.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arvinxx
Copy link
Contributor

arvinxx commented Sep 27, 2024

只需要改文档就好了么?不需要改实现?

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Is it just a matter of changing the document? No need to change the implementation?

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Sep 27, 2024
@EINDEX
Copy link
Contributor Author

EINDEX commented Sep 27, 2024

@arvinxx Sorry, that time is not ready for review.

I added a new SSO module for Microsoft Entra ID and kept the old Azure AD.

For the documentation side, there is no setting up process from Azure AD to Entra ID. Only the env vars and redirect URL path are updated by the Next Auth package.

PS: My mbp m1 pro with 16G RAM is very slow to build this project 😭.

@EINDEX
Copy link
Contributor Author

EINDEX commented Oct 1, 2024

@arvinxx please review this PR

@EINDEX EINDEX changed the title 📝 docs: update entra id sso document ✨ feat: update entra id sso document Oct 1, 2024
src/config/auth.ts Outdated Show resolved Hide resolved
src/libs/next-auth/sso-providers/microsoft-entra-id.ts Outdated Show resolved Hide resolved
src/libs/next-auth/sso-providers/microsoft-entra-id.ts Outdated Show resolved Hide resolved
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 2, 2024
docs/self-hosting/environment-variables/auth.zh-CN.mdx Outdated Show resolved Hide resolved
src/config/auth.ts Outdated Show resolved Hide resolved
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Oct 12, 2024
src/config/auth.ts Outdated Show resolved Hide resolved
@EINDEX
Copy link
Contributor Author

EINDEX commented Oct 19, 2024 via email

@EINDEX
Copy link
Contributor Author

EINDEX commented Oct 19, 2024

@arvinxx please review

Copy link
Contributor

@arvinxx arvinxx left a comment

Choose a reason for hiding this comment

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

LGTM now, Thanks for your contribution!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 20, 2024
Copy link

vercel bot commented Oct 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lobe-chat-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 20, 2024 3:52am

@arvinxx arvinxx changed the title ✨ feat: update entra id sso document ♻️ refactor: refactor azure ad to ms entra id Oct 20, 2024
@arvinxx arvinxx merged commit 4fa9588 into lobehub:main Oct 20, 2024
6 of 7 checks passed
@lobehubbot
Copy link
Member

❤️ Great PR @EINDEX ❤️

The growth of project is inseparable from user feedback and contribution, thanks for your contribution! If you are interesting with the lobehub developer community, please join our discord and then dm @arvinxx or @canisminor1990. They will invite you to our private developer channel. We are talking about the lobe-chat development or sharing ai newsletter around the world.
项目的成长离不开用户反馈和贡献,感谢您的贡献! 如果您对 LobeHub 开发者社区感兴趣,请加入我们的 discord,然后私信 @arvinxx@canisminor1990。他们会邀请您加入我们的私密开发者频道。我们将会讨论关于 Lobe Chat 的开发,分享和讨论全球范围内的 AI 消息。

github-actions bot pushed a commit that referenced this pull request Oct 20, 2024
### [Version&nbsp;1.22.11](v1.22.10...v1.22.11)
<sup>Released on **2024-10-20**</sup>

#### ♻ Code Refactoring

- **misc**: Refactor azure ad to ms entra id.

<br/>

<details>
<summary><kbd>Improvements and Fixes</kbd></summary>

#### Code refactoring

* **misc**: Refactor azure ad to ms entra id, closes [#4168](#4168) ([4fa9588](4fa9588))

</details>

<div align="right">

[![](https://img.shields.io/badge/-BACK_TO_TOP-151515?style=flat-square)](#readme-top)

</div>
@lobehubbot
Copy link
Member

🎉 This PR is included in version 1.22.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

@BrandonStudio
Copy link
Contributor

把环境变量改了是不是应该发个公告啊。。。

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Should an announcement be made after changing the environment variables? . .

github-actions bot pushed a commit to bentwnghk/lobe-chat that referenced this pull request Oct 20, 2024
### [Version&nbsp;1.65.6](v1.65.5...v1.65.6)
<sup>Released on **2024-10-20**</sup>

#### ♻ Code Refactoring

- **misc**: Refactor azure ad to ms entra id.

#### 💄 Styles

- **misc**: Add Llama 3.1 Nemotron 70B model & reorder some provider model list, add Ministral model, update Together AI model list, add function call & vision, update wenxin 4.0 turbo model to latest.

<br/>

<details>
<summary><kbd>Improvements and Fixes</kbd></summary>

#### Code refactoring

* **misc**: Refactor azure ad to ms entra id, closes [lobehub#4168](https://github.com/bentwnghk/lobe-chat/issues/4168) ([4fa9588](4fa9588))

#### Styles

* **misc**: Add Llama 3.1 Nemotron 70B model & reorder some provider model list, closes [lobehub#4424](https://github.com/bentwnghk/lobe-chat/issues/4424) ([9355a3d](9355a3d))
* **misc**: Add Ministral model, closes [lobehub#4427](https://github.com/bentwnghk/lobe-chat/issues/4427) ([2042df8](2042df8))
* **misc**: Update Together AI model list, add function call & vision, closes [lobehub#4393](https://github.com/bentwnghk/lobe-chat/issues/4393) ([d7fbf1b](d7fbf1b))
* **misc**: Update wenxin 4.0 turbo model to latest, closes [lobehub#4428](https://github.com/bentwnghk/lobe-chat/issues/4428) ([3389fbb](3389fbb))

</details>

<div align="right">

[![](https://img.shields.io/badge/-BACK_TO_TOP-151515?style=flat-square)](#readme-top)

</div>
@Ken-Mok
Copy link

Ken-Mok commented Oct 21, 2024

Both Azure AD and Microsoft Entra ID are currently not working.

Azure AD:

  • The tenantId is missing.
  • Authentication is routed to Microsoft common oauth endpoint.
  • The app requires multi-tenant support enabled in the appId.

Microsoft Entra ID:

  • The following parameters are missing: clientId, clientSecret, and tenantId.

@arvinxx
Copy link
Contributor

arvinxx commented Oct 21, 2024

@BrandonStudio 这个是新增的,没有改旧的吧

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@BrandonStudio This is a new addition, not an old one, right?

@arvinxx arvinxx mentioned this pull request Oct 21, 2024
8 tasks
@arvinxx
Copy link
Contributor

arvinxx commented Oct 21, 2024

@BrandonStudio @Ken-Mok #4438

@arvinxx
Copy link
Contributor

arvinxx commented Oct 21, 2024

The following parameters are missing: clientId, clientSecret, and tenantId.

@cy948 Don't MS entra id follow next auth env inference?

@Ken-Mok
Copy link

Ken-Mok commented Oct 21, 2024

@arvinxx
src/config/auth.ts 没有包含MS entra id 的 authenv initilization 代碼
src/libs/next-auth/sso-providers/microsoft-entra-id.ts 没有clientId, clientSecret, and tenantId的代碼.

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@arvinxx
src/config/auth.ts does not contain authenv initilization code for MS entra id
src/libs/next-auth/sso-providers/microsoft-entra-id.ts does not have code for clientId, clientSecret, and tenantId.

@arvinxx
Copy link
Contributor

arvinxx commented Oct 21, 2024

src/libs/next-auth/sso-providers/microsoft-entra-id.ts 没有clientId, clientSecret, and tenantId的代碼.

参考这个 PR ,理论上应该不需要配置 process.env 的

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


src/libs/next-auth/sso-providers/microsoft-entra-id.ts does not have code for clientId, clientSecret, and tenantId.

Refer to this [PR](https://github.com/lobehub/lobe-chat/pull/3701#:~:text=%E5%BE%80%E5%90%8E%E6%8E%A5%E5% 85%A5%20sso%20provider%20%E6%97%B6%E4%B8%8D%E9%9C%80%E8%A6%81%E4%BB%8E%20authEnv%20%E4%B8%AD% E8%AF%BB%E5%8F%96%E5%8F%98%E9%87%8F%EF%BC%8C%E5%BD%93%E9%85%8D%E7%BD%AE%E4% B8%AD%E4%B8%8D%E5%90%ABclientId%2C%20issuer%E6%97%B6next%2Dauth%E6%89%8D%E4%BC%9A%E8%BF%9B%E8%A1% 8Cenv%20infer%EF%BC%8C%E7%A4%BA%E4%BE%8B%EF%BC%9A), theoretically there should be no need to configure process.env

@Ken-Mok
Copy link

Ken-Mok commented Oct 21, 2024

另外,ENV 使用 AUTH_MICROSOFT_ENTRA_ID_ID 和 AUTH_MICROSOFT_ENTRA_ID_SECRET 命名有點特別。
建議使用 AUTH_MICROSOFT_ENTRA_CLIENT_ID 和 AUTH_MICROSOFT_ENTRA_CLIENT_SECRET,或者 AUTH_ENTRA_CLIENT_ID 和 AUTH_ENTRA_CLIENT_SECRET,這樣會比較直觀,因為這些名稱更能清楚地表達它們的用途和含義。

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Also, the ENV naming is a bit special using AUTH_MICROSOFT_ENTRA_ID_ID and AUTH_MICROSOFT_ENTRA_ID_SECRET.
It is recommended to use AUTH_MICROSOFT_ENTRA_CLIENT_ID and AUTH_MICROSOFT_ENTRA_CLIENT_SECRET, or AUTH_ENTRA_CLIENT_ID and AUTH_ENTRA_CLIENT_SECRET, which is more intuitive as these names more clearly convey their purpose and meaning.

@gemnioo
Copy link

gemnioo commented Nov 19, 2024

@BrandonStudio when you fixed this #4723

pls RP included with this updated pics and text in https://github.com/lobehub/lobe-chat/blob/main/docs/self-hosting/advanced/auth/next-auth/microsoft-entra-id.mdx and /microsoft-entra-id.zh-CN.mdx

with this parts:

image

You can fill in or modify the Redirect URIs after registering, but make sure the URL you enter matches the deployed URL. Please replace "your-domain" with your own domain. to

You need make sure the URL enter matches ends "/api/auth/callback/microsoft-entra-id" or you can modify the Redirect URIs by On the app registration page, select Authentication. In the Platform configurations section, select Add URI to add the redirect URI displayed below

请务必保证填写的 URL 后缀是 “/api/auth/callback/microsoft-entra-id” 或者你可以后续在 “App registration -> Authentication ” 路径如下图修改重定向 URL

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝 Documentation Improvements or additions to documentation | 文档问题 lgtm This PR has been approved by a maintainer released size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants