Project

General

Profile

Bug #1736

FBS-bibliotekers brugere kan ikke se infomedia-artikler

Added by Christian Vandel about 4 years ago. Updated about 3 years ago.

Status:
Resolved (tag version)
Priority:
Urgent
Assignee:
Estimated time:
URL med eksempel:
Kategorier:
Søgning - Værkvisning, Søgning - Materialevisning, Integration - FBS, Integration - Brønd - Data og relationer

Description

Det viser sig at være et generelt problem for FBS-biblioteker at man ikke kan se artikler fra infomedia.

I kaldet til useraccessinfomedia sendes fornavn (eller navn?) i stedet for lånerID/CPR. Hvis jeg forstår koden rigtigt henter ting_infomedia oplysningerne ved at kalde ding_user_get_creds(), så det er data i sessionen, som afviger.

Om det er en bug i FBS provideren eller i ting_infomedia må andre afgøre...

Buggen er verificeret i version 2.5.1


Related issues

Has duplicate DDB CMS - Bug #2250: Infomedia artikler ved søgning på vores hjemmesiderClosed

History

#1 Updated by Rolf Madsen about 4 years ago

  • Status changed from New to Ready for development
  • Priority changed from Normal to Urgent
  • Target version set to DDB CMS 2016 2. opgradering
  • Kategorier Søgning - Værkvisning, Søgning - Materialevisning, Integration - Brønd - Data og relationer added

#2 Updated by Rolf Madsen almost 4 years ago

  • Target version changed from DDB CMS 2016 2. opgradering to DDB CMS 2017 1. opgradering (7.x-4.0.2)

#3 Updated by Rolf Madsen over 3 years ago

  • Target version changed from DDB CMS 2017 1. opgradering (7.x-4.0.2) to DDB CMS 2017 1. opgradering (DBC sprintbacklog)

#4 Updated by Rolf Madsen over 3 years ago

  • Assignee set to Laura Holm

#5 Updated by per johansen over 3 years ago

  • Status changed from Ready for development to Development
  • Assignee changed from Laura Holm to per johansen

jeg kigger på denher

#6 Updated by per johansen over 3 years ago

  • Status changed from Development to Needs code review
  • Assignee changed from per johansen to Kasper Garnæs

i både alma og openruth bruges 'name' til brugerens id

$return['creds'] = array(
'name' => $name,
'pass' => $pass,
);

i fbs bruges library_card_number

$result['creds']['library_card_number'] = $name;

Jeg har lavet rettelsen i infomedia modulet, da jeg ikke kan overskue konsekvenserne af at rette i fbs.user - hvis fx. navnet skal vises i en eller anden formular ... eller bliver brugt til reservation eller ...

done a pull-request
https://github.com/ding2/ding2/pull/370

#7 Updated by Jørgen Høst over 3 years ago

Vi har netop konstateret i Silkeborg, at vi også har dette problem efter overgang til Cicero. Dejligt at der ser ud til at være en løsning på vej :-)

#8 Updated by Jesper Kristensen over 3 years ago

  • Status changed from Needs code review to Reviewed - Needs info/rework
  • Assignee changed from Kasper Garnæs to per johansen

Koden ser fin ud, men det kunne være godt med en rebase også lige fjern de to commits der ophæver hinanden.

#9 Updated by per johansen over 3 years ago

  • Status changed from Reviewed - Needs info/rework to Needs code review
  • Assignee changed from per johansen to Jesper Kristensen

så er der rebased

#10 Updated by Gitte Barlach over 3 years ago

  • Assignee changed from Jesper Kristensen to Kasper Garnæs

#11 Updated by Kasper Garnæs over 3 years ago

  • Status changed from Needs code review to Technical test

Reviewed, godkendt og merged.

#12 Updated by Kasper Garnæs over 3 years ago

  • Target version changed from DDB CMS 2017 1. opgradering (DBC sprintbacklog) to DDB CMS 2016 2. opgradering (DBC sprintbacklog)

Jeg har flyttet udgave til 2016.2.

#13 Updated by Lotte Tøstesen over 3 years ago

testet på greve staging. Får fejlmeddelesen "Intern fejl - prøv venligst igen senere"

#14 Updated by Gitte Barlach over 3 years ago

  • Status changed from Technical test to 8
  • Assignee changed from Kasper Garnæs to Gitte Barlach

#15 Updated by Gitte Barlach over 3 years ago

fejler også på upgrade-fbs mod Aarhus driftsmiljø. Både for bruger, der logger ind med lånerkort.nr og brugere der logger ind med NemID.

Fejlmeddelesen lyder: "Intern fejl - prøv venligst igen senere"Det ser ud til at
DB loggen siger: borrower_not_found. SEVERITY notice

Det ser ud til at det er dette forhold, som Fini påpeger i http://platform.dandigbib.org/issues/1343#note-2, der ikke er taget højde for i implementeringen:

"Man kan ikke nøjes med at checke den cachede pinkode, for FBS gemmer ikke pinkoden i cachen.
ding_user skal kalde authenticate i provideren, for den kan ikke vide om provideren cacher pinkoden i creds. Creds kan ligesåvel være en form for session id."

#16 Updated by Gitte Barlach over 3 years ago

  • Status changed from 8 to Reviewed - Needs info/rework
  • Assignee changed from Gitte Barlach to per johansen

kan du se på den igen, Per?

#17 Updated by Christian Vandel over 3 years ago

Autentifikationen foregår i useraccessinfomedia (egentlig borchk), så hvis det skal spille uden cachede pinkoder forudsætter det ændringer på service-siden...

#18 Updated by Rolf Madsen over 3 years ago

@Christian ...

  1. har du nogen idé om hvor stor en ændring af denne art i infomedia servicen er?
  2. vores umiddelbare forslag er at tage behovet for denne ændring i infomedia servicen op i Teknisk koordinationsgruppe når DDB mødes med DBC den 23. januar, kan du se andre veje til en løsning?

#19 Updated by Rolf Madsen over 3 years ago

  • Status changed from Reviewed - Needs info/rework to Kombit FBI (Waiting)
  • Assignee changed from per johansen to Rolf Madsen
  • Target version changed from DDB CMS 2016 2. opgradering (DBC sprintbacklog) to DDB CMS 2017 1. opgradering (7.x-4.0.2)

#20 Updated by per johansen over 3 years ago

findes der en testbruger til fbs ?

#21 Updated by per johansen over 3 years ago

  • Assignee changed from Rolf Madsen to Jesper Kristensen

okay . nu har jeg (endelig) fået det til at virke lokalt. i modsætning til alma og openruth gemmer fbs IKKE pinkode når brugeren logger ind. Med denher lille rettelse (+mit tidligere commit) virker det.

$result['creds']['pass'] = $pincode;

done a pull request
https://github.com/ding2/ding2/pull/514

#22 Updated by Simon Holt over 3 years ago

Fedt, vi har ventet på denne rettelse :)

Vil lige se om jeg kan nå at teste det i morgen.

#23 Updated by Gitte Barlach over 3 years ago

  • Assignee changed from Jesper Kristensen to Kasper Garnæs

#24 Updated by Gitte Barlach over 3 years ago

  • Status changed from Kombit FBI (Waiting) to Needs code review

#25 Updated by Gitte Barlach over 3 years ago

  • Target version changed from DDB CMS 2017 1. opgradering (7.x-4.0.2) to DDB CMS 2016 2. opgradering (DBC sprintbacklog)

#26 Updated by Kasper Garnæs over 3 years ago

  • Status changed from Needs code review to Reviewed - Needs info/rework
  • Assignee changed from Kasper Garnæs to per johansen

Jeg synes det er ærgeligt hvis vi skal begynde at gemme brugernavn og password for brugerne. Det betyder i praksis at hvis en angriber får adgang til session storage så opnås adgang til brugernavn og password, hvilket ikke ellers ville være tilfældet.

Jeg har spurgt Thomas Fini om input.

Hvis vi går med denne løsning skal det som minimum kommenteres bedre.

#27 Updated by per johansen over 3 years ago

nu bliver lånerid jo allerede gemt, og det er vel den der er kritisk, så at gemme pinkoden også ser jeg som en mindre ting.

#28 Updated by Thomas Hansen over 3 years ago

At lånerid bliver gemt skyldes udelukkende at FBS kræver det ved pinkode skift, og der er ingen måder at forespørge det on demand. Grunden til at de kræver det hænger nok sammen med at man jo både kan have et lånerkort og CPR nummer.

Men det er sådan set en bug i FBS som bør fikses og er som udgangspunkt ingen undskyldning for at vi holder på mere sensitiv data.

Men hvis vi lige tager et lidt større overblik, så er Ding jo originalt skrevet med antagelsen af at creds ikke altid er tilgængelig. Der er kode der fanger at ding_get_creds kaster en exception, præsenterer en login form og prøver igen. Ideen var at man kunne forblive logget ind i DDB CMS, og gøre ting der ikke brugte bibliotekssystemet, selvom ens login til bibliotekssystemet var timet ud af sikkerhedshensyn.

Men koden der timer creds ud er fjernet til fordel for autologout, så nu lader det til at være antagelsen at hvis man er logget ind, så er der også creds.

Så det burde sådan set være muligt at lave det så at der bliver bedt om password til FBS når man klikker på en infomedia artikel. Men det er sikkert ikke en løsning der vil blive populær, tror de færreste biblioteksbrugere kan se det rimelige i at de skal give deres password mere end en gang.

Hvilket bringer os tilbage til antagelsen om at hvis man er logget ind i DDB CMS, så er man også logget ind i bibliotekssystemet. Rent filosofisk betyder det at "man" nu stoler mere på DDB CMS end man gjorde engang, for dengang var det vigtigt at Ding2 kun holdt på følsomme oplysninger så kort tid som muligt.

Er det den nye virkelighed? Rolf, Gitte, Jesper, Kasper?

For det ville da gøre nogen ting nemmere, men så mangler der altså lige lidt oprydning. For der er en del kode og skumle tricks der understøtter den gamle virkelighed. F.eks. overrider ding_provider system/ajax for at kunne fange manglende creds og præsentere en login form, og ding_provider_get_form() kloner store dele af drupal_get_form()/drupal_build_form() for at kunne håndtere at provideren siger du ikke er logget ind, midt i en form submission. Det ville simplificere en hel del hvis det blev fjernet.

Sekundært, hvis creds nu som standard får lov at leve i sessionen så længe den lever, var det måske på sin plads at harden'e det en smule. Jeg tænker at hvis vi sætter en cookie med en random key, og bruger den til at kryptere creds i session, så får vi lidt ekstra beskyttelse. Bl.a. at en udvikler der kigger i sessions tabellen på serveren ikke ser folks pinkode i plain-text. Der er ikke nogen magic bullet der beskytter mod alt, men det betyder at et database dump i sig selv ikke er nok til at få adgang til nogens konti.

Så kan vi sådan set godt gemme FBS pinkode i creds, selvom det stadig går imod ideen med at man ikke behøver at gemme creds for at bruge FBS.

Og så er der også det alternativ at forsøge at få FBS til at levere creds.

Men alt i alt er det en temmelig dårlig single signon løsning vi har her. Hvem styrer infomedia? Kunne de ændre på noget?

#29 Updated by Christel Krabbenhøft over 3 years ago

  • Assignee changed from per johansen to Gitte Barlach
  • Target version changed from DDB CMS 2016 2. opgradering (DBC sprintbacklog) to DDB CMS 2017 1. opgradering (7.x-4.0.2)

#30 Updated by Rolf Madsen over 3 years ago

Tak for input og altid skarpe pointer Thomas!

For at sætter der yderligere i perspektiv så har vi netop afsluttet første fase af et projekt til udvikling af den nye Adgangsplatform.

Hvis jeg forstår det du skriver og mulighederne i Adgangsplatformen korrekt vil vi med integration til denne service netop kunne undgå at opbevare credentials i DDB CMS.

Man kan læse mere om Adgangsplatformen under https://login.bib.dk/example/.

Adgangsplatformen er i praksis endnu ikke driftsklar fordi den er afhængig af overførsel af lånerprofilernes tværgående sammenhæng til CULR.

Vi har fået et positivt tilsagn om ad de bliver overført fra FBS inden sommerferien og de sidste DDELibra biblioteker, som er tilsluttet DDB hjemmesidepakken, overgår efter planen til FBS i uge 44 2017.

DBC oplyser at der ligger testdata i CULR så vi kan sådan set påbegynde udviklingen af et nyt login modul i DDB CMS nu.

Med forbehold for at jeg endnu ikke har set input fra Core team vil jeg foreslå en pragmatisk løsning på problemstillingen i dette issue også arbejde for en bedre løsning med integrationen til Adgangsplatformen i 1./2. kvartal af 2017.

#31 Updated by Thomas Hansen over 3 years ago

Nåmen, hvis I bare vil have en hurtig midlertidig løsning, så merger i bare Pers PR.

Men altså... Nok kan login.bib.dk eliminere nødvendigheden af at gemme creds lokalt, men den genopliver behovet for at DDB CMS kan håndtere at man er logget ind i Drupal men ikke i biblioteks systemet. Jeg antager at tickets fra login.bib.dk ikke er evigt gyldige (det ville jo være dumt), så vi er tilbage til at DDB CMS skal være klar på at noget siger "du er ikke logget ind" og håndtere det på en hensigtsmæssig måde...

Hvilket den jo forsøger, men hvis DDB CMS skal til at bruge login.bib.dk, så kommer der nogen voldsomme udfordringer. DDB CMS kan PT håndtere at det sker i et ajax callback (hvilket jo stort set er alle direkte interaktioner såsom reserveringer, PT), men det kan den primært fordi den kan sende en login form som Ajax svar. Det kan den ikke særligt nemt med en ekstern login service.

Så der kommer der lige en ny udfordring der.

#32 Updated by Simon Holt over 3 years ago

Vi bruger den her rettelse i produktion nu, men glæden var kort. Brugere der er logget ind med NemID har jo ingen pinkode i creds og bliver derfor bedt om at logge ind selv om de er logget ind.

#33 Updated by Rolf Madsen about 3 years ago

  • Assignee changed from Gitte Barlach to per johansen

#34 Updated by Gitte Barlach about 3 years ago

Hermed en opdatering af sagen med seneste info. fra DBC:

Infomedia webservicen kræver at brugeren har et lånerid og en pinkode, og det har en bruger der logger ind med NemId ikke.
1) Der skal ændres i selve infomedia webservicen for at udelade pinkodetjek
2) Hvert enkelte bibliotek skal selv oprette en lånertjekmetode i VIP særskilt til infomedia, som udelader pinkodetjek
3) Men det er muligvis ikke nok, det skal undersøges om DDB CMS kan trække et lånerid (cpr) ud af NemiD.
Så det er ikke et nemt fix.

#35 Updated by per johansen about 3 years ago

1. Jeg har lavet et fix i infomedia-webservicen, så den ikke skal bruge pincode. Det skal sættes i drift, og det må jeg ikke. Jeg finder en der gerne må.
2. Der findes allerede en lånertjekmetode, så den behøver bibliotekerne ikke at lave - (skal lige cleares med en eller anden, som ved om vi bruge den)
3. Såvidt jeg kan se er cpr-nummer i creds - lige til at bruge.

Alt i alt (hvis vi må) virker det nemmere end jeg lige troede.

#36 Updated by Gitte Barlach about 3 years ago

Hej Per

Det lyder godt;-)

Såfremt du bliver positivt bekræftet i dine antagelser, skal der da så stadig rettes i DDB CMS koden eller er det overflødigt?

#37 Updated by per johansen about 3 years ago

det tror jeg faktisk ikke, men det må komme an på en prøve

#38 Updated by Gitte Barlach about 3 years ago

  • Status changed from Reviewed - Needs info/rework to Kombit FBI (Waiting)

Mange tak, Per. Det krydser vi fingre for ;-)

#39 Updated by Simon Holt about 3 years ago

@per

Er infomedia fiks kommet i drift? Tænkte jeg lige ville se om jeg kunne nå at teste det i dag.

#40 Updated by per johansen about 3 years ago

desværre ikke - anders vestergaard er en travl mand - jeg har committet til svn og givet besked.

#41 Updated by Simon Holt about 3 years ago

ok, tak for svar.

#42 Updated by Rolf Madsen about 3 years ago

  • Status changed from Kombit FBI (Waiting) to Development

#43 Updated by Rolf Madsen about 3 years ago

  • Related to Bug #2250: Infomedia artikler ved søgning på vores hjemmesider added

#44 Updated by Rolf Madsen about 3 years ago

  • Related to deleted (Bug #2250: Infomedia artikler ved søgning på vores hjemmesider)

#45 Updated by Rolf Madsen about 3 years ago

  • Has duplicate Bug #2250: Infomedia artikler ved søgning på vores hjemmesider added

#46 Updated by Rolf Madsen about 3 years ago

Seneste melding fra DBC er at fejlen har været analyseret og de har en løsning klar til test, men jeg mangler at høre status på hvordan testen er gået.

#47 Updated by Simon Holt about 3 years ago

Har lige testet for DBC og Pers fiks af infomedia-webservice ser ud til at virke.

Testede både login med NemID og almindelig bibliotekslogin. I begge tilfælde fik jeg vist infomedia-artiklen korrekt.

#48 Updated by Gitte Barlach about 3 years ago

Jeg har netop lige testet på stg.aakb.dk og kan bekræfte at det virker, som Simon ganske rigtigt siger:-)

#49 Updated by per johansen about 3 years ago

yeahaaaa

#50 Updated by Rolf Madsen about 3 years ago

  • Status changed from Development to Technical test
  • Assignee changed from per johansen to Gitte Barlach

Kan vi ændre status til Resolved eller skal vi lige have testet på vanilla/upgrade miljøerne?

#51 Updated by per johansen about 3 years ago

først og fremmest skal pull-requesen lige godkendes og merges - simon har fået det til at virke med en eller anden form for patch

#52 Updated by Rolf Madsen about 3 years ago

  • Status changed from Technical test to Development
  • Assignee changed from Gitte Barlach to per johansen

OK, så det er altså en kodeændring i DDB CMS der skal til, og ikke "kun" i Infomedia web servicen?

Lægger du et PR på så vi kan ændre status til Needs code review?

#53 Updated by Rolf Madsen about 3 years ago

Vi har lige testet på https://upgrade-fbs.ddbcms.dk/ting/object/870971-avis%3A80932073# og kan læse Infomedia artiklen både med CPR-nr./pinkode OG NemID!

#54 Updated by per johansen about 3 years ago

gad vide om den er blevet merget ?? -

#55 Updated by Rolf Madsen about 3 years ago

Per kan du afklare hvor det merge skulle være sket?

Er det et merge i forhold til:

  1. Infomedia servicen
  2. DDB CMS?

#56 Updated by Gitte Barlach about 3 years ago

jeg kan ikke se at denne er merget: https://github.com/ding2/ding2/pull/514
men det skal den vel heller ikke?

#57 Updated by per johansen about 3 years ago

det var dether:
https://github.com/ding2/ding2/pull/370

og det er blevet merget

#58 Updated by Rolf Madsen about 3 years ago

  • Status changed from Development to Resolved (tag version)
  • Target version changed from DDB CMS 2017 1. opgradering (7.x-4.0.2) to DDB CMS 2016 2. opgradering

Dette issue lukkes hermed med status Resolved (tag version) og Version ændres også da koden allerede er merged i version DDB CMS 2016 2. opgradering.

Also available in: Atom PDF