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

Auth methode #2

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Auth methode #2

merged 1 commit into from
Feb 6, 2023

Conversation

outscale-mgo
Copy link
Contributor

No description provided.

return -1;
}

if (strlen(e->ak) != AK_SIZE || strlen(e->sk) != SK_SIZE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

strnlen_s ?

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'm not sure what it would check, I already check that ak/sk are not null.
if the string lack a 0 termination, then there is already nothing I can do.

So adding those check would only be useful, if there is an non-null-terminated string that go beyond strsz.
but I'm not sure the positive of this would overcome the negative of adding a C11 functions that might be less supported than strlen. (C11 glibc support is not the same as C11 compiler support)

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 made this issue outscale/osc-sdk-c#23

lib.c Outdated
}
} else if (e->auth_method == OSC_PASSWORD_METHOD) {
e->ak = getenv("OSC_LOGIN");
e->sk = getenv("OSC_PASSWORD");
Copy link
Contributor

Choose a reason for hiding this comment

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

could you use dedicated struct element like e->login and e->password to avoid confusion and future errors?

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 could but that would duplicate the code too.
i could use a

union {
char *ak;
char *login;
}

but anonymous union are not C89 and header, try to be C89.

}
curl_easy_setopt(e->c, CURLOPT_HTTPHEADER, e->headers);
curl_easy_setopt(e->c, CURLOPT_USERNAME, e->ak);
curl_easy_setopt(e->c, CURLOPT_PASSWORD, e->sk);
Copy link
Contributor

Choose a reason for hiding this comment

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

Even if e->auth_method == OSC_AKSK_METHOD ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, ak/sk use CURLOPT_USERNAME/CURLOPT_PASSWORD for the ak and sk.
the curl_easy_setopt(e->c, CURLOPT_AWS_SIGV4, "osc"); is for v4 specific arguments.

lib.h Outdated
@@ -69,13 +69,19 @@ struct osc_str {
#define OSC_API_VERSION "____api_version____"
#define OSC_SDK_VERSION ____sdk_version____

enum osc_auth_method {
OSC_AKSK_METHODE,
Copy link
Contributor

Choose a reason for hiding this comment

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

-> OSC_AKSK_METHOD

lib.c Outdated

*password = NULL;
*email = NULL;
strcpy(stpcpy(buf, home), dest);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe check home size as buf is bounded to 1024

lib.c Outdated
if (auth && (!strcmp(auth, "password") || !strcmp(auth, "basic")))
e->auth_method = OSC_PASSWORD_METHOD;
else if (auth && strcmp(auth, "accesskey")) {
fprintf(stderr, "'%s' invalude auth methode\n", auth);
Copy link
Contributor

Choose a reason for hiding this comment

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

-> invalid authentication method

Signed-off-by: Matthias Gatto <[email protected]>
@outscale-mgo outscale-mgo merged commit 6c620a7 into master Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants