Skip to content

Conversation

@kungasc
Copy link
Contributor

@kungasc kungasc commented Dec 18, 2024

Changelog entry

...

Changelog category

Check SID existence in ACL operation

Additional information

https://ydb.yandex-team.ru/docs/concepts/internal/iam#avtorizaciya

@kungasc kungasc self-assigned this Dec 18, 2024
@github-actions
Copy link

github-actions bot commented Dec 18, 2024

2024-12-18 10:24:08 UTC Pre-commit check linux-x86_64-relwithdebinfo for 5dc049a has started.
2024-12-18 10:24:21 UTC Artifacts will be uploaded here
2024-12-18 10:27:47 UTC ya make is running...
🟡 2024-12-18 12:19:07 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
31130 27422 0 377 3192 139

2024-12-18 12:22:22 UTC ya make is running... (failed tests rerun, try 2)
🟡 2024-12-18 12:57:57 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1146 (only retried tests) 307 0 395 299 145

2024-12-18 12:58:25 UTC ya make is running... (failed tests rerun, try 3)
🔴 2024-12-18 13:32:18 UTC Some tests failed, follow the links below.

Test history | Ya make output | Test bloat | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1066 (only retried tests) 230 0 394 297 145

🟢 2024-12-18 13:32:31 UTC Build successful.
🟢 2024-12-18 13:33:24 UTC ydbd size 2.1 GiB changed* by +43.0 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 2940f2f merge: 5dc049a diff diff %
ydbd size 2 253 716 864 Bytes 2 253 760 896 Bytes +43.0 KiB +0.002%
ydbd stripped size 483 049 840 Bytes 483 056 496 Bytes +6.5 KiB +0.001%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 18, 2024

2024-12-18 10:24:15 UTC Pre-commit check linux-x86_64-release-asan for 5dc049a has started.
2024-12-18 10:24:26 UTC Artifacts will be uploaded here
2024-12-18 10:27:35 UTC ya make is running...
🟡 2024-12-18 12:07:50 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14161 13626 0 325 143 67

🟢 2024-12-18 12:09:05 UTC Build successful.
🟢 2024-12-18 12:09:33 UTC ydbd size 3.6 GiB changed* by +66.3 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 2940f2f merge: 5dc049a diff diff %
ydbd size 3 894 517 928 Bytes 3 894 585 792 Bytes +66.3 KiB +0.002%
ydbd stripped size 1 363 834 448 Bytes 1 363 849 936 Bytes +15.1 KiB +0.001%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@kungasc kungasc force-pushed the check-user-existstance branch from 52c8f75 to e2c7642 Compare December 19, 2024 07:14
@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 07:16:14 UTC Pre-commit check linux-x86_64-release-asan for 309414b has started.
2024-12-19 07:16:25 UTC Artifacts will be uploaded here
2024-12-19 07:19:18 UTC ya make is running...
🔴 2024-12-19 07:24:44 UTC Build failed, see the logs. Also see fail summary

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 07:17:35 UTC Pre-commit check linux-x86_64-relwithdebinfo for 309414b has started.
2024-12-19 07:17:47 UTC Artifacts will be uploaded here
2024-12-19 07:20:50 UTC ya make is running...
🔴 2024-12-19 07:24:14 UTC Build failed, see the logs. Also see fail summary

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 08:28:06 UTC Pre-commit check linux-x86_64-relwithdebinfo for 97b8e76 has started.
2024-12-19 08:28:18 UTC Artifacts will be uploaded here
2024-12-19 08:31:24 UTC ya make is running...
🟡 2024-12-19 09:47:18 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
19377 17093 0 374 1773 137

2024-12-19 09:49:30 UTC ya make is running... (failed tests rerun, try 2)
🟡 2024-12-19 10:24:34 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1138 (only retried tests) 298 0 396 299 145

2024-12-19 10:24:48 UTC ya make is running... (failed tests rerun, try 3)
🔴 2024-12-19 11:00:10 UTC Some tests failed, follow the links below.

Test history | Ya make output | Test bloat | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1068 (only retried tests) 231 0 395 297 145

🟢 2024-12-19 11:00:24 UTC Build successful.
🟢 2024-12-19 11:01:06 UTC ydbd size 2.1 GiB changed* by +7.2 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 5244b5d merge: 97b8e76 diff diff %
ydbd size 2 254 802 928 Bytes 2 254 810 272 Bytes +7.2 KiB +0.000%
ydbd stripped size 483 250 224 Bytes 483 251 248 Bytes +1.0 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 08:28:20 UTC Pre-commit check linux-x86_64-release-asan for 97b8e76 has started.
2024-12-19 08:28:32 UTC Artifacts will be uploaded here
2024-12-19 08:31:33 UTC ya make is running...
🟡 2024-12-19 09:46:57 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
12161 11642 0 305 144 70

🟢 2024-12-19 09:48:25 UTC Build successful.
🟢 2024-12-19 09:48:54 UTC ydbd size 3.6 GiB changed* by +12.8 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 5244b5d merge: 97b8e76 diff diff %
ydbd size 3 896 411 120 Bytes 3 896 424 256 Bytes +12.8 KiB +0.000%
ydbd stripped size 1 364 406 704 Bytes 1 364 408 944 Bytes +2.2 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

azevaykin
azevaykin previously approved these changes Dec 19, 2024
@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 13:21:27 UTC Pre-commit check linux-x86_64-release-asan for 07e0e9f has started.
2024-12-19 13:21:31 UTC Artifacts will be uploaded here
2024-12-19 13:22:34 UTC Check cancelled

Copy link
Collaborator

@azevaykin azevaykin left a comment

Choose a reason for hiding this comment

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

This check should be only for local users, not external like @ldap.

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 13:24:09 UTC Pre-commit check linux-x86_64-relwithdebinfo for 3d701d2 has started.
2024-12-19 13:24:20 UTC Artifacts will be uploaded here
2024-12-19 13:24:57 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 13:24:13 UTC Pre-commit check linux-x86_64-release-asan for 3d701d2 has started.
2024-12-19 13:24:23 UTC Artifacts will be uploaded here
2024-12-19 13:24:57 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 13:26:19 UTC Pre-commit check linux-x86_64-relwithdebinfo for a4d1689 has started.
2024-12-19 13:26:23 UTC Artifacts will be uploaded here
2024-12-19 13:29:21 UTC ya make is running...
🟡 2024-12-19 14:38:09 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
31141 28169 0 25 2831 116

2024-12-19 14:41:15 UTC ya make is running... (failed tests rerun, try 2)
🟡 2024-12-19 14:52:47 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
203 (only retried tests) 72 0 19 0 112

2024-12-19 14:52:56 UTC ya make is running... (failed tests rerun, try 3)
🔴 2024-12-19 15:04:43 UTC Some tests failed, follow the links below.

Test history | Ya make output | Test bloat | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
190 (only retried tests) 60 0 18 0 112

🟢 2024-12-19 15:04:49 UTC Build successful.
🟢 2024-12-19 15:05:11 UTC ydbd size 2.1 GiB changed* by +6.0 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 8de259d merge: a4d1689 diff diff %
ydbd size 2 234 567 104 Bytes 2 234 573 264 Bytes +6.0 KiB +0.000%
ydbd stripped size 477 802 096 Bytes 477 803 312 Bytes +1.2 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

2024-12-19 13:27:44 UTC Pre-commit check linux-x86_64-release-asan for a4d1689 has started.
2024-12-19 13:28:09 UTC Artifacts will be uploaded here
2024-12-19 13:31:34 UTC ya make is running...
🟡 2024-12-19 14:51:31 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14159 14067 0 35 9 48

🟢 2024-12-19 14:52:49 UTC Build successful.
🟢 2024-12-19 14:53:12 UTC ydbd size 3.6 GiB changed* by +10.2 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 8de259d merge: a4d1689 diff diff %
ydbd size 3 865 219 832 Bytes 3 865 230 280 Bytes +10.2 KiB +0.000%
ydbd stripped size 1 350 327 376 Bytes 1 350 329 872 Bytes +2.4 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 20, 2024

2024-12-20 10:18:57 UTC Pre-commit check linux-x86_64-relwithdebinfo for e98a3cd has started.
2024-12-20 10:19:42 UTC Artifacts will be uploaded here
2024-12-20 10:23:11 UTC ya make is running...
🟡 2024-12-20 11:34:46 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
31153 28202 0 4 2832 115

2024-12-20 11:37:20 UTC ya make is running... (failed tests rerun, try 2)
🟡 2024-12-20 11:49:41 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
167 (only retried tests) 55 0 1 1 110

2024-12-20 11:49:50 UTC ya make is running... (failed tests rerun, try 3)
🔴 2024-12-20 12:01:30 UTC Some tests failed, follow the links below.

Test history | Ya make output | Test bloat | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
154 (only retried tests) 45 0 1 0 108

🟢 2024-12-20 12:01:38 UTC Build successful.
🟢 2024-12-20 12:01:59 UTC ydbd size 2.1 GiB changed* by +5.9 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 291222e merge: e98a3cd diff diff %
ydbd size 2 234 869 408 Bytes 2 234 875 496 Bytes +5.9 KiB +0.000%
ydbd stripped size 477 835 088 Bytes 477 836 176 Bytes +1.1 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 20, 2024

2024-12-20 10:19:03 UTC Pre-commit check linux-x86_64-release-asan for e98a3cd has started.
2024-12-20 10:19:07 UTC Artifacts will be uploaded here
2024-12-20 10:22:03 UTC ya make is running...
🟡 2024-12-20 11:43:42 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14162 14084 0 21 7 50

🟢 2024-12-20 11:44:53 UTC Build successful.
🟢 2024-12-20 11:45:15 UTC ydbd size 3.6 GiB changed* by +10.4 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 291222e merge: e98a3cd diff diff %
ydbd size 3 865 748 784 Bytes 3 865 759 432 Bytes +10.4 KiB +0.000%
ydbd stripped size 1 350 424 624 Bytes 1 350 427 312 Bytes +2.6 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@kungasc kungasc force-pushed the check-user-existstance branch from 39c40aa to 4a502bd Compare December 20, 2024 13:41
@github-actions
Copy link

github-actions bot commented Dec 20, 2024

2024-12-20 13:44:54 UTC Pre-commit check linux-x86_64-release-asan for dc5fa9d has started.
2024-12-20 13:45:06 UTC Artifacts will be uploaded here
2024-12-20 13:48:12 UTC ya make is running...
🟡 2024-12-20 15:15:48 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14173 14096 0 24 7 46

🟢 2024-12-20 15:17:14 UTC Build successful.
🟢 2024-12-20 15:17:39 UTC ydbd size 3.6 GiB changed* by +10.4 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 31a14ef merge: dc5fa9d diff diff %
ydbd size 3 866 243 400 Bytes 3 866 254 040 Bytes +10.4 KiB +0.000%
ydbd stripped size 1 350 509 584 Bytes 1 350 512 272 Bytes +2.6 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 20, 2024

2024-12-20 13:45:07 UTC Pre-commit check linux-x86_64-relwithdebinfo for dc5fa9d has started.
2024-12-20 13:45:19 UTC Artifacts will be uploaded here
2024-12-20 13:48:28 UTC ya make is running...
🟡 2024-12-20 15:02:58 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
31164 28215 0 2 2831 116

2024-12-20 15:06:01 UTC ya make is running... (failed tests rerun, try 2)
🟢 2024-12-20 15:17:44 UTC Tests successful.

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
162 (only retried tests) 53 0 0 0 109

🟢 2024-12-20 15:17:51 UTC Build successful.
🟢 2024-12-20 15:18:12 UTC ydbd size 2.1 GiB changed* by +17.9 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: a580b00 merge: dc5fa9d diff diff %
ydbd size 2 235 533 360 Bytes 2 235 551 688 Bytes +17.9 KiB +0.001%
ydbd stripped size 477 922 896 Bytes 477 927 440 Bytes +4.4 KiB +0.001%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@kungasc kungasc requested a review from azevaykin December 20, 2024 16:41
@kungasc kungasc marked this pull request as ready for review December 20, 2024 16:46
@kungasc kungasc requested a review from a team as a code owner December 20, 2024 16:46
return result;
}

if (acl) {
Copy link
Member

Choose a reason for hiding this comment

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

these checks don't make any sense:

  1. the idea behind YDB ACLs is that you can grant permissions to any SID (whether it exists or not). This is intentional, as a user can be created after their permissions are set.
  2. You can remove a user after this check, making it redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

bool TLoginProvider::CheckSidExistsOrIsNonYdb(const TString& sid) {
// non-YDB user's sid format is <login>@<subsystem>
Copy link
Member

Choose a reason for hiding this comment

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

there are no rules on SID formats. it's intentional.
it could be anything. and even in the login library you could have SIDs with @ symbol.
and it has nothing to do with YDB or any other project that uses this library.

Copy link
Contributor Author

@kungasc kungasc Dec 20, 2024

Choose a reason for hiding this comment

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

there are no rules on SID formats. it's intentional.

You can't create neither a user nor a group with @ symbol in its name

if (!CheckAllowedName(request.User)) {

Copy link
Member

Choose a reason for hiding this comment

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

okay, I see, but anyway, I think this check looks very weak and I don't think it should have place in the abstract login library. please, keep it abstract, it could be reused anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved check out of the library

@github-actions
Copy link

github-actions bot commented Dec 20, 2024

2024-12-20 17:33:13 UTC Pre-commit check linux-x86_64-relwithdebinfo for 569ec91 has started.
2024-12-20 17:33:24 UTC Artifacts will be uploaded here
2024-12-20 17:36:16 UTC ya make is running...
🟡 2024-12-20 18:27:46 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
19402 17874 0 1 1415 112

2024-12-20 18:29:29 UTC ya make is running... (failed tests rerun, try 2)
🟢 2024-12-20 18:41:15 UTC Tests successful.

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
158 (only retried tests) 49 0 0 0 109

🟢 2024-12-20 18:41:23 UTC Build successful.
🟢 2024-12-20 18:41:39 UTC ydbd size 2.1 GiB changed* by +8.6 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: d2e4bb8 merge: 569ec91 diff diff %
ydbd size 2 235 568 584 Bytes 2 235 577 432 Bytes +8.6 KiB +0.000%
ydbd stripped size 477 930 768 Bytes 477 931 920 Bytes +1.1 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions
Copy link

github-actions bot commented Dec 20, 2024

2024-12-20 17:33:38 UTC Pre-commit check linux-x86_64-release-asan for 569ec91 has started.
2024-12-20 17:33:49 UTC Artifacts will be uploaded here
2024-12-20 17:36:40 UTC ya make is running...
🟡 2024-12-20 18:37:52 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
12176 12101 0 19 8 48

🟢 2024-12-20 18:39:24 UTC Build successful.
🟢 2024-12-20 18:39:46 UTC ydbd size 3.6 GiB changed* by +15.5 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: d2e4bb8 merge: 569ec91 diff diff %
ydbd size 3 866 288 592 Bytes 3 866 304 512 Bytes +15.5 KiB +0.000%
ydbd stripped size 1 350 523 280 Bytes 1 350 526 160 Bytes +2.8 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@kungasc kungasc merged commit d543026 into ydb-platform:main Dec 26, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants