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

Configurazione dinamica OIDC member #123

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

MdreW
Copy link
Collaborator

@MdreW MdreW commented Feb 6, 2025

Ciao @mspasiano , ho riportato qui la tua pull request provando ad evitare anche l'eval.
La mia ipotesi è questa:

  • Dai claim OIDC riceviamo esclusivamente stringhe (eventualmente formattate)
  • La stringa dovrà essere trasformata in un booleano
  • Un valore booleano nel DB dovrà essere sempre inserito (default false)

Quindi ho creato due ENV, uno che definisce il claim da leggere (default member), un altro che definisca il valore con cui deve essere confrontato per essere considerato valido (default "true").

Ho anche modificato la sorgente del claim dal dato raw a quello elaborato "info". Dimmi se ti piace come soluzione

Copy link
Collaborator

@mspasiano mspasiano left a comment

Choose a reason for hiding this comment

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

Nel mio caso no va perchè l'attributo member è presente nell'ogetto auth molto annidato es. "auth.extra.raw_info.is_cnr_user" e non ho trovato modo di leggerlo se non tramite un eval

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 6, 2025

Effettivamente l'assegnazione dello stato member può richiedere elaborazioni anche complesse, più che un eval potremmo pensare ad una libreria esterna fatta appositamente per essere personalizzata senza dolore es:

Creiamo il file `lib/custom/oidc.rb

class Oidc
  def self.member?(auth)
    # my special receipt:
    # auth.extra.raw_info.try(:is_member) || auth.extra.groups.include?('users')
    false
  end
end

e nel file app/model/users.rb modifichiamo:

class User < ApplicationRecord
  user.member = Custom::Oidc.member? auth
end

Per chi vuole utilizzare la funzionalità non dovrà fare altro che fare il mount in docker di una nuova versione del file, magari possiamo mettere il commento di questo mount con le istruzioni nel dockerfile.

Non si creano sovrapposizioni perché quel file non ha altre funzioni ed eliminiamo gli env specifici. Ti può piacere quest'approccio?

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 6, 2025

Volendo potremmo estrarre anche in una libreria esterna l'intera parte OIDC (inclusi gli ENV di configurazione) in modo da permettere la modifica di un sistema che facilmente richiede personalizzazioni:

app/model/user.rb

class User < ApplicationRecord
  ...
   # @return user finded or created from omiauth session
  def self.from_omniauth(auth)
    Custom::Oidc.new find_or_initialize_by(username: auth.uid), auth 
  end
  ....
end

lib/custom/oidc.rb

class Custom::Oidc
  def initialize(user, auth)
    user.email = auth.info.email
    user.password = SecureRandom.alphanumeric(20)
    user.name = auth.info.try(ENV.fetch("RAILS_OIDC_NAME", "given_name" ))
    user.surname = auth.info.try(ENV.fetch("RAILS_OIDC_SURNAME", "family_name" ))
    user.member = Custom::Affiliation.new auth
    user.skip_confirmation! if RAILS_DEVISE_CONFIRMABLE
    user.save
    user
  end
end

lib/custom/affiliation.rb

class Custom::Affiliation
  def initialize(auth)
    # my special receipt:
    # auth.extra.raw_info.try(:is_member) || auth.extra.groups.include?('users')
    auth.info.try(ENV.fetch("RAILS_OIDC_MEMBER", "member")) == ENV.fetch("RAILS_OIDC_MEMBER_VALUE", "true")
  end
end

A scapito di un po di spezzatino dovremmo riuscire a rendere facile la sostituzione dei vari metodi senza andare a modificare il core del sito... ma magari la sto facendo più complessa del necessario 😄
P.S. Codice scritto a memoria, non testato!

@mspasiano
Copy link
Collaborator

Volendo potremmo estrarre anche in una libreria esterna l'intera parte OIDC (inclusi gli ENV di configurazione) in modo da permettere la modifica di un sistema che facilmente richiede personalizzazioni:

app/model/user.rb

class User < ApplicationRecord
  ...
   # @return user finded or created from omiauth session
  def self.from_omniauth(auth)
    Custom::Oidc.new find_or_initialize_by(username: auth.uid), auth 
  end
  ....
end

lib/custom/oidc.rb

class Custom::Oidc
  def initialize(user, auth)
    user.email = auth.info.email
    user.password = SecureRandom.alphanumeric(20)
    user.name = auth.info.try(ENV.fetch("RAILS_OIDC_NAME", "given_name" ))
    user.surname = auth.info.try(ENV.fetch("RAILS_OIDC_SURNAME", "family_name" ))
    user.member = Custom::Affiliation.new auth
    user.skip_confirmation! if RAILS_DEVISE_CONFIRMABLE
    user.save
    user
  end
end

lib/custom/affiliation.rb

class Custom::Affiliation
  def initialize(auth)
    # my special receipt:
    # auth.extra.raw_info.try(:is_member) || auth.extra.groups.include?('users')
    auth.info.try(ENV.fetch("RAILS_OIDC_MEMBER", "member")) == ENV.fetch("RAILS_OIDC_MEMBER_VALUE", "true")
  end
end

A scapito di un po di spezzatino dovremmo riuscire a rendere facile la sostituzione dei vari metodi senza andare a modificare il core del sito... ma magari la sto facendo più complessa del necessario 😄 P.S. Codice scritto a memoria, non testato!

Direi che può andare bene, così posso anche fare override di un singolo pezzo se lo ritengo opportuno, di contro però l'innesto deve essere fatto prima di creare l'immagine docker, per cui devo avere poi un'immagine personalizzata e non posso utilizzare ad esempio quella ufficiale e lavorare sulle ENV per le personalizzazioni, o sbaglio?

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 6, 2025

No, puoi mettere gli eventuali mount nel dockerfile per sovrascrivere quello che ti è necessario. Il vantaggio è che non vai a sovrascrivere il core del sistema ma soltanto dei file con funzioni specifiche

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 6, 2025

Oggi son un po incasinato, appena mi avanza un secondo lo metto nella request, così lo provi nella realtà

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 6, 2025

@mspasiano , ho inserito il nuovo codice lasciando commentata la vecchia versione, prova a dare un occhiata.
Ho inserito nel docker-compose un esempio dei mount, al momento fanno riferimento ai file già presenti nel repository, ma nessuno ti impedisce di metterli esterni o in un volume esterno per poterli modificare in diretta,

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 6, 2025

Magari rinominerei la classe che si occupa di creare / aggiornare gli utenti OIDC in OidcUser, giusto per dargli un nome "parlante" come ho fatto per affiliation

@MdreW
Copy link
Collaborator Author

MdreW commented Feb 10, 2025

@mspasiano, ho approvato la tua correzione, appena mi dai l'ok facciamo merge e passiamo a migliorare le estrazioni 👍

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.

OIDC Configurazione dinamica del campo member in base ad un attributo via ENV
2 participants