feat: update Yandex and VK OIDC #4158
Conversation
| if len(userInfo.User.Phone) > 0 && userInfo.User.Phone[0] != '+' { | ||
| userInfo.User.Phone = "+" + userInfo.User.Phone |
There was a problem hiding this comment.
🤔 I'm not sure if this is the correct normalization to do here. I quickly checked VK docs, but couldn't find anything about the format.
If it is not clear, I'd prefer to do nothing instead and leave it to the consumer of the data.
There was a problem hiding this comment.
All I've got is two examples from the docs, as well as a confirmation that it works that way (I've tested on my VK account that is registered with a Russian phone number). I didn't find any detailed description of the format either.
The examples from the docs:
{
"user": {
"user_id": "1234567890", // Идентификатор пользователя.
"first_name": "Ivan", // Имя пользователя.
"last_name": "I.", // Первая буква фамилии пользвателя.
"phone": "79903922415", // Телефон пользователя.
"avatar": "https://pp.userapi.com/60tZWMo4SmwcploUVl9XEt8ufnTTvDUmQ6Bj1g/mmv1pcj63C4.png", // Ссылка на фото профиля пользователя.
"email": "[email protected]", // Адрес почты пользователя.
"sex": 2, // Пол. Возможные значения: Возможные значения: 1 — женский, 2 — мужской, 0 — пол не указан.
"is_verified": false, // Признак того, что аккаунт пользователя подтвержден. Если это так, возвращается параметр true.
"birthday": "01.01.2000" // Дата рождения пользователя.
}
}{
"user": {
"user_id": "1234567890",
"first_name": "Ivan",
"last_name": "I.",
"phone": "79991234567",
"avatar": "http://avatar.com/12345678",
"email": "[email protected]",
"sex": 2,
"verified": false,
"birthday": "01.01.2000"
}
}79903922415 and 79991234567 are Russian phone numbers in the international format, but without a leading + sign.
As far as I understand, Ory Kratos uses international format, but with a leading + sign.
I'm not sure whether all the phones are returned in the same format or not. For example I can't test with non-Russian phones. This is why I added condition if there's no plus sign, then add it.
If you insist, I can remove that code block (lines 162-164). It shouldn't be the problem to do the same action within a .jsonnet file though.
Changes
VK provider: Support PKCE
It seems like VK doesn't provide
/.well-known/openid-configuration(VK doesn't support OpenID).So to use PKCE it's needed to be enabled with:
VK provider: Support returning
phone_numberinclaimsVK offers different endpoints for PKCE and non-PKCE configuration.
Token endpoint that is used in non-PKCE configuration can return
emailbut notphone.Extra endpoint
https://id.vk.com/oauth2/user_infocan be used to retrieveemailandphone, but it doesn't acceptaccess_tokenthat was returned from non-PKCE token endpoint.So to retrieve a phone, PKCE is required to be enabled:
- id: vk provider: vk mapper_url: "data-mapper.jsonnet" + pkce: "force" scope: - email + - phoneAdditional info:
I added
AccessTokenURLOptions(r *http.Request) []oauth2.AuthCodeOptionto OAuth2Provider interface.It works like
AuthCodeURLOptions(r ider) []oauth2.AuthCodeOptionbut it's dedicated to update a token URL.VK requires passing
device_idURL param (that is received by a callback) to PKCE token endpoint. Without this change it's impossible.I updated all the other OIDC providers to comply with the updated interface.
Yandex provider: Support returning
phone_numberinclaimsNo extra configuration is needed.
Related issue(s)
Fixes: #4147
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments