Using /admin/users/sync_sso endpoint to sync SSO data, it is not possible to remove someone’s name from the account, even if it is not required. In other other words, it’s not possible to change an account’s name from something to nothing. This is happening with full_name_required = false (and sso_overrides_name = true).
I believe the issue is here:
but I’m afraid my knowledge of Ruby/Discourse isn’t that hot to submit a PR.
This feels like a feature request to me. At the moment you can use the admin API to clear names in cases like this. See:
Sorry, but I disagree. I don’t see how this can be a feature request and not a bug.
I understand there’s other API endpoints which can be used. But the main point of /admin/users/sync_sso is precisely to keep this sort of data, well, in sync. It already allows to set the account name field — that works. But, it assumes name is always a required field and doesn’t allow it to be set it to blank. So it can’t be used to keep data in sync.
The code below seems to make it work as expected on my sandbox, but again, I don’t feel confident enough to submit a PR at this point. Feel free to use it/adapt it, etc.
- if SiteSetting.sso_overrides_name && user.name != name && name.present?
- user.name = name || User.suggest_name(username.blank? ? email : username)
+ if SiteSetting.sso_overrides_name && user.name != name
+ if SiteSetting.full_name_required && name.present?
+ user.name = name || User.suggest_name(username.blank? ? email : username)
+ else
+ user.name = name
+ end
Its a feature request, the semantics are ambiguous. Lack of name could mean
You are asking for a protocol change here. I usually prefer explicitness so we don’t have a confusing api.
I see your point now.
Once again, I don’t know the internals (or ruby) enough… but there’s no way to know if the field name was provided on that request? Not present at all, don’t touch the name field of the account. Field name provided, set it (if blank, clear it). Wouldn’t this respect that protocol/behavior where only the provided fields are synched?
Sorry if this makes no sense — just trying to grasp from what I understand.
The problem is that the protocol semantics now of “name” param is there and set to blank, is the same as no “name” param. It simply does not touch it.
A change here is a change to protocol semantics. We could I guess change is so name= becomes name is blanked, and absence of name means “don’t touch name”, but people already rely on the old behavior so this is technically a breaking change.
Can you explain why you are removing names?
We are not removing everyone’s names — the users are updating their own data whenever they update their account on our website (SSO provider). We rely on /admin/users/sync_sso to keep data in sync (username, name, avatar, etc) in their Discourse account. Name is an optional field and can be set to blank/empty.
I just realized the issue happens not just with name but also with updating bio, avatar, etc: it’s not possible to keep those SSO records in sync through /admin/users/sync_sso if they need to be updated to blank/empty, regardless whether they are mandatory fields or not.
I understand the point that people may be relying on existing behavior (although nobody reported this issue before?), but if this is the protocol, it seems it has significant limitations for its purpose of synchronizing SSO records.
I’m also getting bitten by this, individual users are unable to remove their own personal information (name, avatars, bio, custom fields etc) from Discourse which has ramifications as you can imagine.
Agree with not changing the semantics of how it works now but could you not allow us to set these attributes as false in the SSO payload or something similar to be explicit about removing them?
Anteriormente, estávamos contornando essa limitação do SSO compondo a chamada para o endpoint /admin/users/sync_sso com outra chamada para o endpoint /u/{username} apenas para limpar o nome (se o novo valor para o nome estivesse vazio).
No entanto, isso também parece ter deixado de funcionar em alguma versão recente, possivelmente porque verifica se sso_overrides_name = true antes de atualizar o nome.
Portanto, como está, ao usar SSO e sso_overrides_name = true, agora parece impossível para o provedor de SSO limpar o campo de nome no Discourse via API.
Você consegue ver alguma solução alternativa para isso, @sam?
Acho que um parâmetro extra para nossa rota sync_sso seria apropriado? Algo como &clear_name não tenho certeza. Isso parece um caso extremo. Qual é o caso de uso para nomes em branco? Talvez apenas definir como nome de usuário se você não tiver nenhum e a interface do usuário puder suprimir duplicação.
É confuso para mim que você considere isso um caso extremo, então imagino que você esteja acostumado com instâncias onde o nome é o campo obrigatório.
Nós temos o contrário: o nome de usuário é o que todos devem ter, e o nome é um campo opcional (usamos prioritize_username_in_ux = true e full name required = false). Pense nas contas do Twitter, onde todos devem ter um nome de usuário/handle, e podem opcionalmente ter um nome também. Não acho que seja um cenário de caso extremo querer limpar o campo de nome (ou outros dados pessoais).
No momento, uma vez que alguém preencheu um nome, é impossível removê-lo. Isso era uma limitação do sync_sso que contornamos com uma chamada extra de API para atualizar o usuário, mas agora isso também não funciona mais.
Consideramos isso, mas induz algumas pessoas a assumir que o nome de usuário de alguém é também seu nome real: administramos um fórum internacional e muitas vezes não fica claro o que é o nome de uma pessoa e o que é um nome de usuário (além de sua posição na interface).
Devo mencionar que, se a memória não me falha, o mesmo problema exato acontece ao remover o avatar de alguém através do sync_sso, pois acho que isso também não funciona — contornamos isso fornecendo nossa própria URL para um avatar padrão.
Se houver vários campos que um usuário não possa limpar de alguma forma, talvez um array (ou lista csv) de quais campos limpar/resetar?
O que seria necessário para chegar a um acordo sobre uma forma de realizar isso? Ficarei feliz em implementar qualquer coisa do lado: um novo parâmetro, um valor especial, uma segunda chamada de API. Mas a situação atual, da minha perspectiva, não é um caso extremo. Como @mentalstring acima, estou sincronizando com uma fonte externa de verdade onde definir uma foto de perfil e um nome de exibição são opcionais. O usuário pode não tê-los. O Discourse permite que eles não sejam definidos. Se você não usa SSO, pode defini-los e removê-los livremente. O DiscourseConnect quebra isso: uma vez definidos, eles nunca podem ser removidos, o que, na minha opinião, é um (muito pequeno) bug.
Entendo a preocupação em alterar o protocolo onde vazio atualmente significa nenhuma alteração. Pessoalmente, discordo, acho que é uma maneira direta e razoável de interpretar o protocolo e o risco é minúsculo: seria muito estranho que sistemas fossem implementados de forma que sempre enviassem os valores de nome e avatar_url, mas às vezes os definissem como em branco, e esperassem que isso significasse manter o valor antigo. E se houver algum que faça isso, a consequência é apenas que o nome e o avatar sejam desdefinidos, e deve ser uma correção fácil.
Mas, de qualquer forma, não quero discutir esse ponto, mas quero defender que este é um recurso necessário. Estou disposto a fazer o PR, só quero saber qual solução seria aceita.
Muito obrigado desde já.
Dado que isso acontece com pelo menos name e avatar_url e possivelmente outros (acho que website também?), talvez um parâmetro reset_fields com uma lista de campos para limpar em vez de vários clear_x individuais?
Para nós, já seria útil se pudéssemos corrigir com chamadas adicionais ao endpoint /u/{username}, mas isso também parou de funcionar em algum momento.