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

Add groups #559

Merged
merged 19 commits into from
May 14, 2024
Merged

Add groups #559

merged 19 commits into from
May 14, 2024

Conversation

NikitaUnisikhin
Copy link
Collaborator

-- added group authentication

sources/group.h Outdated
char *storage_db;

char *group_name;
int group_name_len;
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that this field of structure exists only in order to exist :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

@@ -0,0 +1,32 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

add "#ifndef" + "define" for new headers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

sources/group.h Outdated

int od_group_free(od_group_t *);
void od_group_qry_format(char *, char *, ...);
int od_group_parse_val_datarow(machine_msg_t *, int *);
Copy link
Contributor

Choose a reason for hiding this comment

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

add "#endif" here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

sources/rules.c Outdated
{
/* Allocate and force defaults */
od_group_t *group;
group = (od_group_t *)malloc(sizeof(*group));
Copy link
Contributor

Choose a reason for hiding this comment

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

no need cast it to (od_group_t *), in C you don't need it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

sources/rules.c Outdated
group = (od_group_t *)malloc(sizeof(*group));
if (group == NULL)
return NULL;
memset(group, 0, sizeof(*group));
Copy link
Contributor

Choose a reason for hiding this comment

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

use calloc() at line 132 (its faster + the clearest way to allocate a buffer which filled with 0)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

/* (not used) */
uint32_t val_len;
rc = kiwi_read32(&val_len, &pos, &pos_size);
if (kiwi_unlikely(rc == -1)) {
Copy link
Contributor

@AndrewOvvv AndrewOvvv Feb 9, 2024

Choose a reason for hiding this comment

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

same issue as at line 59 (read from same position)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

why is this better?

goto error;
/* count */
uint16_t count;
rc = kiwi_read16(&count, &pos, &pos_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure that you should read from pos? you have already read 4 bytes of "size" variable from here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in kiwi_read16 the pos pointer changes

Copy link
Contributor

Choose a reason for hiding this comment

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

oh okay, my mistake


/* (not used) */
uint32_t val_len;
rc = kiwi_read32(&val_len, &pos, &pos_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

val_len is the same to size because you read it from same position

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in kiwi_read16 the pos pointer changes

@@ -1203,7 +1209,8 @@ od_config_reader_ldap_storage_credentials(od_config_reader_t *reader,
static int od_config_reader_rule_settings(od_config_reader_t *reader,
od_rule_t *rule,
od_extention_t *extentions,
od_storage_watchdog_t *watchdog)
od_storage_watchdog_t *watchdog,
od_group_t *group)
Copy link
Contributor

Choose a reason for hiding this comment

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

why you gives group as argument? you have already had it in "od_rule_t *rule"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

NULL);
}

static int od_config_reader_group(od_config_reader_t *reader, char *db_name,
Copy link
Contributor

Choose a reason for hiding this comment

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

default way to write and use od_config_reader_something it is to give reader and pointer to struct/variable where you should read

look at od_config_reader_string or od_config_reader_number64 or od_config_reader_keyword

lets give in arguments od_group_t* ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

return NOT_OK_RESPONSE;
}

machine_sleep(1000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we sleep here?


pool_routing "internal"
pool "session"
group_query "select pg_has_role('%s', 'group1', 'MEMBER')"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think %s is an injection

Copy link
Collaborator

@mialinx mialinx Mar 12, 2024

Choose a reason for hiding this comment

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

Do we need to specify group_query at all ?
We may just set as default during configuration, using group name for current route.
Is there any alternatives to this one ^^

@x4m x4m merged commit 8ba1e23 into yandex:master May 14, 2024
0 of 2 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.

4 participants