You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "dev-lpq (via GitHub)" <gi...@apache.org> on 2023/04/06 08:18:17 UTC
[GitHub] [kyuubi] dev-lpq opened a new pull request, #4669: [KYUUBI #4152]Support LDAP authentication for domain users
dev-lpq opened a new pull request, #4669:
URL: https://github.com/apache/kyuubi/pull/4669
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
Now, with binduser and bindpw enabled, the user authentication pass is incompatible with the domain user.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] dev-lpq commented on pull request #4669: LDAP authentication allows auth user contains domain when bind.dn/pw enabled
Posted by "dev-lpq (via GitHub)" <gi...@apache.org>.
dev-lpq commented on PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#issuecomment-1499888711
The PR check issue, how to solve it?
Caused by: org.apache.kyuubi.jdbc.hive.KyuubiSQLException: org.apache.kyuubi.shade.org.apache.thrift.transport.TTransportException: java.net.SocketException: Broken pipe (Write failed)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on pull request #4669: LDAP authentication allows auth user contains domain when bind.dn/pw enabled
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#issuecomment-1499911199
The failures were caused by network issue, thanks, merged to master/1.7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on pull request #4669: [KYUUBI #4152]Support LDAP authentication for domain users
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#issuecomment-1498670990
would you please supply an UT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] codecov-commenter commented on pull request #4669: LDAP authentication allows auth user contains domain when bind.dn/pw enabled
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#issuecomment-1498959116
## [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4669?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#4669](https://codecov.io/gh/apache/kyuubi/pull/4669?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d365bdc) into [master](https://codecov.io/gh/apache/kyuubi/commit/1241a3891431cda602269bd49dcb8621ad77bbb5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1241a38) will **decrease** coverage by `0.04%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head d365bdc differs from pull request most recent head b4d1587. Consider uploading reports for the commit b4d1587 to get more accurate results
```diff
@@ Coverage Diff @@
## master #4669 +/- ##
============================================
- Coverage 57.61% 57.57% -0.04%
Complexity 13 13
============================================
Files 579 579
Lines 31966 31967 +1
Branches 4270 4271 +1
============================================
- Hits 18417 18406 -11
- Misses 11785 11797 +12
Partials 1764 1764
```
| [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4669?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uthentication/LdapAuthenticationProviderImpl.scala](https://codecov.io/gh/apache/kyuubi/pull/4669?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0xkYXBBdXRoZW50aWNhdGlvblByb3ZpZGVySW1wbC5zY2FsYQ==) | `78.04% <100.00%> (ø)` | |
| [...kyuubi/service/authentication/ldap/LdapUtils.scala](https://codecov.io/gh/apache/kyuubi/pull/4669?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL2xkYXAvTGRhcFV0aWxzLnNjYWxh) | `87.80% <100.00%> (+0.30%)` | :arrow_up: |
... and [7 files with indirect coverage changes](https://codecov.io/gh/apache/kyuubi/pull/4669/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 closed pull request #4669: LDAP authentication allows auth user contains domain when bind.dn/pw enabled
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #4669: LDAP authentication allows auth user contains domain when bind.dn/pw enabled
URL: https://github.com/apache/kyuubi/pull/4669
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] dev-lpq commented on a diff in pull request #4669: [KYUUBI #4152]Support LDAP authentication for domain users
Posted by "dev-lpq (via GitHub)" <gi...@apache.org>.
dev-lpq commented on code in PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#discussion_r1159453878
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtils.scala:
##########
@@ -105,12 +105,25 @@ object LdapUtils extends Logging {
* </pre>
*
* @param userName username
- * @return true if `userName`` contains `@<domain>` part
+ * @return true if `userName` contains `@<domain>` part
*/
def hasDomain(userName: String): Boolean = {
ServiceUtils.indexOfDomainMatch(userName) > 0
}
+ /**
+ * Get the username part in the provided user.
+ * <br>
+ * <b>Example:</b>
+ * <br>
+ * For user "user1@mycorp.com"" this method will return "user1"
Review Comment:
ok. will upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4669: [KYUUBI #4152]Support LDAP authentication for domain users
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#discussion_r1159450023
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtils.scala:
##########
@@ -105,12 +105,25 @@ object LdapUtils extends Logging {
* </pre>
*
* @param userName username
- * @return true if `userName`` contains `@<domain>` part
+ * @return true if `userName` contains `@<domain>` part
*/
def hasDomain(userName: String): Boolean = {
ServiceUtils.indexOfDomainMatch(userName) > 0
}
+ /**
+ * Get the username part in the provided user.
+ * <br>
+ * <b>Example:</b>
+ * <br>
+ * For user "user1@mycorp.com"" this method will return "user1"
Review Comment:
nit: duplicated "
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] dev-lpq commented on pull request #4669: [KYUUBI #4152]Support LDAP authentication for domain users
Posted by "dev-lpq (via GitHub)" <gi...@apache.org>.
dev-lpq commented on PR #4669:
URL: https://github.com/apache/kyuubi/pull/4669#issuecomment-1498678101
> would you please supply an UT?
Yes, I will add.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org