all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] fix #3038: check user before renewing ticket
@ 2020-10-20  9:29 Dylan Whyte
  2020-10-21  6:43 ` Dietmar Maurer
  0 siblings, 1 reply; 3+ messages in thread
From: Dylan Whyte @ 2020-10-20  9:29 UTC (permalink / raw)
  To: pbs-devel

Fixes a bug in which the userid of the ticket cache is updated,
when a user connects, but the ticket itself is not.
This means a newly connected user has a previously connected
user's ticket and thus, cannot do anything, as the client will
attempt to use the invalid ticket.

e.g. if john@pbs connected to the server first, followed by
mike@pbs, the following would be stored in the ticket cache.

{
  "localhost": {
    "mike@pbs": {
      "ticket": "PBS:john@pbs:AAAA",
      "timestamp": 1601039326,
      "token": "BBBB"
    }
  }
}

Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
---
 src/client/http_client.rs | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index e3d18604..02a58c2d 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -219,11 +219,13 @@ fn store_ticket_info(prefix: &str, server: &str, username: &str, ticket: &str, t
 
     let empty = serde_json::map::Map::new();
     for (server, info) in data.as_object().unwrap_or(&empty) {
-        for (_user, uinfo) in info.as_object().unwrap_or(&empty) {
-            if let Some(timestamp) = uinfo["timestamp"].as_i64() {
-                let age = now - timestamp;
-                if age < ticket_lifetime {
-                    new_data[server][username] = uinfo.clone();
+        for (user, uinfo) in info.as_object().unwrap_or(&empty) {
+            if user == username {
+                if let Some(timestamp) = uinfo["timestamp"].as_i64() {
+                    let age = now - timestamp;
+                    if age < ticket_lifetime {
+                        new_data[server][username] = uinfo.clone();
+                    }
                 }
             }
         }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] fix #3038: check user before renewing ticket
  2020-10-20  9:29 [pbs-devel] [PATCH proxmox-backup] fix #3038: check user before renewing ticket Dylan Whyte
@ 2020-10-21  6:43 ` Dietmar Maurer
  2020-10-21  7:52   ` Dylan Whyte
  0 siblings, 1 reply; 3+ messages in thread
From: Dietmar Maurer @ 2020-10-21  6:43 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dylan Whyte

This fix is not really correct. I applied it wit a followup patch.
Please can you test my fix?

comments inline:

> On 10/20/2020 11:29 AM Dylan Whyte <d.whyte@proxmox.com> wrote:
> 
>  
> Fixes a bug in which the userid of the ticket cache is updated,
> when a user connects, but the ticket itself is not.
> This means a newly connected user has a previously connected
> user's ticket and thus, cannot do anything, as the client will
> attempt to use the invalid ticket.
> 
> e.g. if john@pbs connected to the server first, followed by
> mike@pbs, the following would be stored in the ticket cache.
> 
> {
>   "localhost": {
>     "mike@pbs": {
>       "ticket": "PBS:john@pbs:AAAA",
>       "timestamp": 1601039326,
>       "token": "BBBB"
>     }
>   }
> }
> 
> Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
> ---
>  src/client/http_client.rs | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/src/client/http_client.rs b/src/client/http_client.rs
> index e3d18604..02a58c2d 100644
> --- a/src/client/http_client.rs
> +++ b/src/client/http_client.rs
> @@ -219,11 +219,13 @@ fn store_ticket_info(prefix: &str, server: &str, username: &str, ticket: &str, t
>  
>      let empty = serde_json::map::Map::new();
>      for (server, info) in data.as_object().unwrap_or(&empty) {
> -        for (_user, uinfo) in info.as_object().unwrap_or(&empty) {
> -            if let Some(timestamp) = uinfo["timestamp"].as_i64() {
> -                let age = now - timestamp;
> -                if age < ticket_lifetime {
> -                    new_data[server][username] = uinfo.clone();
> +        for (user, uinfo) in info.as_object().unwrap_or(&empty) {
> +            if user == username {

not needed

> +                if let Some(timestamp) = uinfo["timestamp"].as_i64() {
> +                    let age = now - timestamp;
> +                    if age < ticket_lifetime {
> +                        new_data[server][username] = uinfo.clone();

 new_data[server][user] = uinfo.clone();

> +                    }
>                  }
>              }
>          }
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] fix #3038: check user before renewing ticket
  2020-10-21  6:43 ` Dietmar Maurer
@ 2020-10-21  7:52   ` Dylan Whyte
  0 siblings, 0 replies; 3+ messages in thread
From: Dylan Whyte @ 2020-10-21  7:52 UTC (permalink / raw)
  To: Dietmar Maurer, Proxmox Backup Server development discussion

All working now, thanks!
> On 21.10.2020 08:43 Dietmar Maurer <dietmar@proxmox.com> wrote:
> 
>  
> This fix is not really correct. I applied it wit a followup patch.
> Please can you test my fix?
> 
> comments inline:
> 
> > On 10/20/2020 11:29 AM Dylan Whyte <d.whyte@proxmox.com> wrote:
> > 
> >  
> > Fixes a bug in which the userid of the ticket cache is updated,
> > when a user connects, but the ticket itself is not.
> > This means a newly connected user has a previously connected
> > user's ticket and thus, cannot do anything, as the client will
> > attempt to use the invalid ticket.
> > 
> > e.g. if john@pbs connected to the server first, followed by
> > mike@pbs, the following would be stored in the ticket cache.
> > 
> > {
> >   "localhost": {
> >     "mike@pbs": {
> >       "ticket": "PBS:john@pbs:AAAA",
> >       "timestamp": 1601039326,
> >       "token": "BBBB"
> >     }
> >   }
> > }
> > 
> > Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
> > ---
> >  src/client/http_client.rs | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/client/http_client.rs b/src/client/http_client.rs
> > index e3d18604..02a58c2d 100644
> > --- a/src/client/http_client.rs
> > +++ b/src/client/http_client.rs
> > @@ -219,11 +219,13 @@ fn store_ticket_info(prefix: &str, server: &str, username: &str, ticket: &str, t
> >  
> >      let empty = serde_json::map::Map::new();
> >      for (server, info) in data.as_object().unwrap_or(&empty) {
> > -        for (_user, uinfo) in info.as_object().unwrap_or(&empty) {
> > -            if let Some(timestamp) = uinfo["timestamp"].as_i64() {
> > -                let age = now - timestamp;
> > -                if age < ticket_lifetime {
> > -                    new_data[server][username] = uinfo.clone();
> > +        for (user, uinfo) in info.as_object().unwrap_or(&empty) {
> > +            if user == username {
> 
> not needed
> 
> > +                if let Some(timestamp) = uinfo["timestamp"].as_i64() {
> > +                    let age = now - timestamp;
> > +                    if age < ticket_lifetime {
> > +                        new_data[server][username] = uinfo.clone();
> 
>  new_data[server][user] = uinfo.clone();
> 
> > +                    }
> >                  }
> >              }
> >          }
> > -- 
> > 2.20.1
> > 
> > 
> > 
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-10-21  7:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20  9:29 [pbs-devel] [PATCH proxmox-backup] fix #3038: check user before renewing ticket Dylan Whyte
2020-10-21  6:43 ` Dietmar Maurer
2020-10-21  7:52   ` Dylan Whyte

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal